Stop exposing the raw eMMC partition as USB mass storage

kvmapp/system/init.d/S03usbdev

@@ -141,17 +141,15 @@ start_usb_dev(){
             fi
             echo "NanoKVM USB Mass Storage0520" > functions/mass_storage.disk0/lun.0/inquiry_string
             disk=$(cat /boot/usb.disk0)
-            if [ -z "${disk}" ]
+            if [ -n "${disk}" ]
             then
-                    # if [ ! -e /mnt/usbdisk.img ]
-                    # then
-                    #         fallocate -l 8G /mnt/usbdisk.img
-                    #         mkfs.vfat /mnt/usbdisk.img
-                    # fi
-                    echo /dev/mmcblk0p3 > functions/mass_storage.disk0/lun.0/file
-            else
                     cat /boot/usb.disk0 > functions/mass_storage.disk0/lun.0/file
             fi
+            # With usb.disk0 empty, leave lun.0/file unset so the gadget
+            # reports "no media" (removable=1) and a BIOS moves on to the next
+            # boot device. This used to point at /dev/mmcblk0p3, handing the
+            # target machine our raw eMMC partition -- which has no MBR, so
+            # Legacy BIOS reads no 0x55AA signature and hangs in a HLT loop.
     fi
 
     ls /sys/class/udc/ | cat > UDC

server/service/storage/image.go

@@ -18,11 +18,13 @@ import (
 
 const (
 	imageDirectory = "/data"
-	imageNone      = "/dev/mmcblk0p3"
-	cdromFlag      = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/cdrom"
-	mountDevice    = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/file"
-	inquiryString  = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/inquiry_string"
-	roFlag         = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/ro"
+	// legacyNoImageDevice is what older builds wrote to mean "no image". It is
+	// only ever read now, never written.
+	legacyNoImageDevice = "/dev/mmcblk0p3"
+	cdromFlag           = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/cdrom"
+	mountDevice         = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/file"
+	inquiryString       = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/inquiry_string"
+	roFlag              = "/sys/kernel/config/usb_gadget/g0/functions/mass_storage.disk0/lun.0/ro"
 )
 
 func (s *Service) GetImages(c *gin.Context) {
@@ -54,6 +56,27 @@ func (s *Service) GetImages(c *gin.Context) {
 	log.Debugf("get images success, total %d", len(images))
 }
 
+// writeMountTarget points the gadget's backing file at an image. An empty
+// image means "no media": the caller has already cleared the backing file, and
+// naming any device here would expose it to the target machine.
+func writeMountTarget(path string, image string) error {
+	if image == "" {
+		return nil
+	}
+	return os.WriteFile(path, []byte(image), 0o666)
+}
+
+// normalizeMountedImage reads back what the gadget is currently serving.
+// Devices that have not rebooted since this change still hold the old eMMC
+// fallback, which has to keep reading as "nothing mounted".
+func normalizeMountedImage(content string) string {
+	image := strings.TrimSpace(content)
+	if image == legacyNoImageDevice {
+		return ""
+	}
+	return image
+}
+
 func (s *Service) MountImage(c *gin.Context) {
 	var req proto.MountImageReq
 	var rsp proto.Response
@@ -109,13 +132,8 @@ func (s *Service) MountImage(c *gin.Context) {
 	}
 
 	// mount
-	image := req.File
-	if image == "" {
-		image = imageNone
-	}
-
-	if err := os.WriteFile(mountDevice, []byte(image), 0o666); err != nil {
-		log.Errorf("mount file %s failed: %s", image, err)
+	if err := writeMountTarget(mountDevice, req.File); err != nil {
+		log.Errorf("mount file %s failed: %s", req.File, err)
 		rsp.ErrRsp(c, -2, "mount image failed")
 		return
 	}
@@ -169,10 +187,7 @@ func (s *Service) GetMountedImage(c *gin.Context) {
 		return
 	}
 
-	image := strings.ReplaceAll(string(content), "\n", "")
-	if image == imageNone {
-		image = ""
-	}
+	image := normalizeMountedImage(string(content))
 
 	data := &proto.GetMountedImageRsp{
 		File: image,

server/service/storage/image_test.go

@@ -0,0 +1,61 @@
+package storage
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+func TestWriteMountTargetLeavesTheGadgetEmptyWhenUnmounting(t *testing.T) {
+	// Unmounting clears the backing file first. Writing a fallback device
+	// afterwards handed the target machine our raw eMMC partition, which has
+	// no MBR -- Legacy BIOS then hangs instead of skipping the device.
+	path := filepath.Join(t.TempDir(), "file")
+	if err := os.WriteFile(path, []byte("\n"), 0o666); err != nil {
+		t.Fatalf("setup: %s", err)
+	}
+
+	if err := writeMountTarget(path, ""); err != nil {
+		t.Fatalf("expected unmounting to succeed: %s", err)
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("expected the backing file to be readable: %s", err)
+	}
+
+	if string(data) != "\n" {
+		t.Fatalf("backing file became %q, want it left cleared", data)
+	}
+}
+
+func TestWriteMountTargetMountsARealImage(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "file")
+
+	if err := writeMountTarget(path, "/data/ubuntu.iso"); err != nil {
+		t.Fatalf("expected mounting to succeed: %s", err)
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("expected the backing file to be readable: %s", err)
+	}
+
+	if string(data) != "/data/ubuntu.iso" {
+		t.Fatalf("backing file is %q, want the image path", data)
+	}
+}
+
+func TestNormalizeMountedImageReportsLegacyEmmcAsNoImage(t *testing.T) {
+	// Devices that have not rebooted since the update still have the eMMC
+	// path in the gadget, and the UI must not show it as a mounted image.
+	if got := normalizeMountedImage("/dev/mmcblk0p3\n"); got != "" {
+		t.Fatalf("normalizeMountedImage = %q, want empty", got)
+	}
+}
+
+func TestNormalizeMountedImageKeepsARealImage(t *testing.T) {
+	if got := normalizeMountedImage("/data/ubuntu.iso\n"); got != "/data/ubuntu.iso" {
+		t.Fatalf("normalizeMountedImage = %q, want the image path", got)
+	}
+}