2019-12-08 17:00:44

by Moritz Müller

[permalink] [raw]
Subject: [PATCH] floppy: hide invalid floppy disk types

In some cases floppy disks are recognised even though no such device
exists. In our case this has been caused by the CMOS-RAM having a few
wrong bits. This caused a non-existent floppy disk with the type 13
(for example) to be registered as an available device, even though it
could not be mounted by any user.

We believe this to be an instance of this bug:

https://bugzilla.kernel.org/show_bug.cgi?id=13486
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/384579

This patch adds the option FLOPPY_ALLOW_UNKNOWN_TYPES to prevent the
additional check that fixed the issue on our reference system, and
increases the startup time of affected systems by over a minute.

Co-developed-by: Philip K. <[email protected]>
Signed-off-by: Philip K. <[email protected]>
Signed-off-by: Moritz Müller <[email protected]>
---
drivers/block/Kconfig | 10 ++++++++++
drivers/block/floppy.c | 6 ++++++
2 files changed, 16 insertions(+)

diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
index 1bb8ec575352..9e6b32c50b67 100644
--- a/drivers/block/Kconfig
+++ b/drivers/block/Kconfig
@@ -72,6 +72,16 @@ config AMIGA_Z2RAM
To compile this driver as a module, choose M here: the
module will be called z2ram.

+config FLOPPY_ALLOW_UNKNOWN_TYPES
+ bool "Allow floppy disks of unknown type to be registered."
+ default n
+ help
+ Select this option if you want the Kernel to register floppy
+ disks of an unknown type.
+
+ This should usually not be enabled, because of cases where the
+ system falsely recongizes a non-existent floppy disk as mountable.
+
config CDROM
tristate
select BLK_SCSI_REQUEST
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index 485865fd0412..9439444d46d0 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3949,7 +3949,9 @@ static void __init config_types(void)
} else
allowed_drive_mask &= ~(1 << drive);
} else {
+#ifdef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
params = &default_drive_params[0].params;
+#ifdef
snprintf(temparea, sizeof(temparea),
"unknown type %d (usb?)", type);
name = temparea;
@@ -4518,6 +4520,10 @@ static bool floppy_available(int drive)
return false;
if (fdc_state[FDC(drive)].version == FDC_NONE)
return false;
+#ifndef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
+ if (type >= ARRAY_SIZE(default_drive_params))
+ return false;
+#endif
return true;
}

--
2.20.1


2019-12-08 17:37:06

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH] floppy: hide invalid floppy disk types

Hi,

one small typo:

On 12/8/19 8:59 AM, Moritz Müller wrote:
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 1bb8ec575352..9e6b32c50b67 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -72,6 +72,16 @@ config AMIGA_Z2RAM
> To compile this driver as a module, choose M here: the
> module will be called z2ram.
>
> +config FLOPPY_ALLOW_UNKNOWN_TYPES
> + bool "Allow floppy disks of unknown type to be registered."
> + default n
> + help
> + Select this option if you want the Kernel to register floppy
> + disks of an unknown type.
> +
> + This should usually not be enabled, because of cases where the
> + system falsely recongizes a non-existent floppy disk as mountable.

recognizes

> +
> config CDROM
> tristate
> select BLK_SCSI_REQUEST


--
~Randy

2019-12-08 19:03:51

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] floppy: hide invalid floppy disk types

Hi "Moritz,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on block/for-next]
[also build test ERROR on linux/master linus/master v5.4 next-20191208]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]

url: https://github.com/0day-ci/linux/commits/Moritz-M-ller/floppy-hide-invalid-floppy-disk-types/20191209-010317
base: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: alpha-defconfig (attached as .config)
compiler: alpha-linux-gcc (GCC) 7.5.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.5.0 make.cross ARCH=alpha

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>

All errors (new ones prefixed by >>):

drivers/block/floppy.c: In function 'config_types':
>> drivers/block/floppy.c:3954:0: error: unterminated #ifdef
#ifdef

drivers/block/floppy.c:3952:0: error: unterminated #ifdef
#ifdef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES

>> drivers/block/floppy.c:3951:3: error: expected declaration or statement at end of input
} else {
^
>> drivers/block/floppy.c:3951:3: error: expected declaration or statement at end of input
drivers/block/floppy.c:3942:8: warning: unused variable 'temparea' [-Wunused-variable]
char temparea[32];
^~~~~~~~
>> drivers/block/floppy.c:3951:3: error: expected declaration or statement at end of input
} else {
^
drivers/block/floppy.c:3925:7: warning: unused variable 'has_drive' [-Wunused-variable]
bool has_drive = false;
^~~~~~~~~
drivers/block/floppy.c: At top level:
drivers/block/floppy.c:563:12: warning: 'floppy_request_regions' declared 'static' but never defined [-Wunused-function]
static int floppy_request_regions(int);
^~~~~~~~~~~~~~~~~~~~~~
drivers/block/floppy.c:564:13: warning: 'floppy_release_regions' declared 'static' but never defined [-Wunused-function]
static void floppy_release_regions(int);
^~~~~~~~~~~~~~~~~~~~~~
drivers/block/floppy.c:565:12: warning: 'floppy_grab_irq_and_dma' declared 'static' but never defined [-Wunused-function]
static int floppy_grab_irq_and_dma(void);
^~~~~~~~~~~~~~~~~~~~~~~
drivers/block/floppy.c:566:13: warning: 'floppy_release_irq_and_dma' declared 'static' but never defined [-Wunused-function]
static void floppy_release_irq_and_dma(void);
^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/block/floppy.c:3923:20: warning: 'config_types' defined but not used [-Wunused-function]
static void __init config_types(void)
^~~~~~~~~~~~
drivers/block/floppy.c:3585:12: warning: 'fd_ioctl' defined but not used [-Wunused-function]
static int fd_ioctl(struct block_device *bdev, fmode_t mode,
^~~~~~~~
drivers/block/floppy.c:3368:12: warning: 'fd_getgeo' defined but not used [-Wunused-function]
static int fd_getgeo(struct block_device *bdev, struct hd_geometry *geo)
^~~~~~~~~
drivers/block/floppy.c:2887:21: warning: 'floppy_queue_rq' defined but not used [-Wunused-function]
static blk_status_t floppy_queue_rq(struct blk_mq_hw_ctx *hctx,
^~~~~~~~~~~~~~~
drivers/block/floppy.c:505:13: warning: 'floppy_device_name' defined but not used [-Wunused-variable]
static char floppy_device_name[] = "floppy";
^~~~~~~~~~~~~~~~~~
drivers/block/floppy.c:418:30: warning: 'tag_sets' defined but not used [-Wunused-variable]
static struct blk_mq_tag_set tag_sets[N_DRIVE];
^~~~~~~~
drivers/block/floppy.c:417:24: warning: 'disks' defined but not used [-Wunused-variable]
static struct gendisk *disks[N_DRIVE];
^~~~~
drivers/block/floppy.c:254:12: warning: 'irqdma_allocated' defined but not used [-Wunused-variable]
static int irqdma_allocated;
^~~~~~~~~~~~~~~~
In file included from drivers/block/floppy.c:252:0:
arch/alpha/include/asm/floppy.h:81:12: warning: 'FDC2' defined but not used [-Wunused-variable]
static int FDC2 = -1;
^~~~
arch/alpha/include/asm/floppy.h:80:12: warning: 'FDC1' defined but not used [-Wunused-variable]
static int FDC1 = 0x3f0;
^~~~
drivers/block/floppy.c:211:12: warning: 'can_use_virtual_dma' defined but not used [-Wunused-variable]
static int can_use_virtual_dma = 2;
^~~~~~~~~~~~~~~~~~~
drivers/block/floppy.c:209:12: warning: 'FLOPPY_IRQ' defined but not used [-Wunused-variable]
static int FLOPPY_IRQ = 6;
^~~~~~~~~~

vim +3954 drivers/block/floppy.c

3922
3923 static void __init config_types(void)
3924 {
3925 bool has_drive = false;
3926 int drive;
3927
3928 /* read drive info out of physical CMOS */
3929 drive = 0;
3930 if (!UDP->cmos)
3931 UDP->cmos = FLOPPY0_TYPE;
3932 drive = 1;
3933 if (!UDP->cmos)
3934 UDP->cmos = FLOPPY1_TYPE;
3935
3936 /* FIXME: additional physical CMOS drive detection should go here */
3937
3938 for (drive = 0; drive < N_DRIVE; drive++) {
3939 unsigned int type = UDP->cmos;
3940 struct floppy_drive_params *params;
3941 const char *name = NULL;
3942 char temparea[32];
3943
3944 if (type < ARRAY_SIZE(default_drive_params)) {
3945 params = &default_drive_params[type].params;
3946 if (type) {
3947 name = default_drive_params[type].name;
3948 allowed_drive_mask |= 1 << drive;
3949 } else
3950 allowed_drive_mask &= ~(1 << drive);
> 3951 } else {
3952 #ifdef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
3953 params = &default_drive_params[0].params;
> 3954 #ifdef
3955 snprintf(temparea, sizeof(temparea),
3956 "unknown type %d (usb?)", type);
3957 name = temparea;
3958 }
3959 if (name) {
3960 const char *prepend;
3961 if (!has_drive) {
3962 prepend = "";
3963 has_drive = true;
3964 pr_info("Floppy drive(s):");
3965 } else {
3966 prepend = ",";
3967 }
3968
3969 pr_cont("%s fd%d is %s", prepend, drive, name);
3970 }
3971 *UDP = *params;
3972 }
3973
3974 if (has_drive)
3975 pr_cont("\n");
3976 }
3977

---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation


Attachments:
(No filename) (6.78 kB)
.config.gz (12.89 kB)
Download all attachments

2019-12-10 05:08:16

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH] floppy: hide invalid floppy disk types

On 12/8/19 9:59 AM, Moritz Müller wrote:
> In some cases floppy disks are recognised even though no such device
> exists. In our case this has been caused by the CMOS-RAM having a few
> wrong bits. This caused a non-existent floppy disk with the type 13
> (for example) to be registered as an available device, even though it
> could not be mounted by any user.
>
> We believe this to be an instance of this bug:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=13486
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/384579
>
> This patch adds the option FLOPPY_ALLOW_UNKNOWN_TYPES to prevent the
> additional check that fixed the issue on our reference system, and
> increases the startup time of affected systems by over a minute.
>
> Co-developed-by: Philip K. <[email protected]>
> Signed-off-by: Philip K. <[email protected]>
> Signed-off-by: Moritz Müller <[email protected]>
> ---
> drivers/block/Kconfig | 10 ++++++++++
> drivers/block/floppy.c | 6 ++++++
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 1bb8ec575352..9e6b32c50b67 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -72,6 +72,16 @@ config AMIGA_Z2RAM
> To compile this driver as a module, choose M here: the
> module will be called z2ram.
>
> +config FLOPPY_ALLOW_UNKNOWN_TYPES
> + bool "Allow floppy disks of unknown type to be registered."
> + default n
> + help
> + Select this option if you want the Kernel to register floppy
> + disks of an unknown type.
> +
> + This should usually not be enabled, because of cases where the
> + system falsely recongizes a non-existent floppy disk as mountable.
> +
> config CDROM
> tristate
> select BLK_SCSI_REQUEST
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 485865fd0412..9439444d46d0 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -3949,7 +3949,9 @@ static void __init config_types(void)
> } else
> allowed_drive_mask &= ~(1 << drive);
> } else {
> +#ifdef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
> params = &default_drive_params[0].params;
> +#ifdef

Please don't send patches that haven't even been compiled.

--
Jens Axboe