linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] floppy: hide invalid floppy disk types
@ 2019-12-08 16:59 Moritz Müller
  2019-12-08 17:35 ` Randy Dunlap
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Moritz Müller @ 2019-12-08 16:59 UTC (permalink / raw)
  To: linux-kernel, linux-block, linux-kernel; +Cc: Moritz Müller, Philip K .

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. <philip@warpmail.net>
Signed-off-by: Philip K. <philip@warpmail.net>
Signed-off-by: Moritz Müller <moritzm.mueller@posteo.de>
---
 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
  2019-12-08 16:59 [PATCH] floppy: hide invalid floppy disk types Moritz Müller
@ 2019-12-08 17:35 ` Randy Dunlap
  2019-12-08 19:01 ` kbuild test robot
  2019-12-10  5:07 ` Jens Axboe
  2 siblings, 0 replies; 9+ messages in thread
From: Randy Dunlap @ 2019-12-08 17:35 UTC (permalink / raw)
  To: Moritz Müller, linux-kernel, linux-block, linux-kernel; +Cc: Philip K .

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
  2019-12-08 16:59 [PATCH] floppy: hide invalid floppy disk types Moritz Müller
  2019-12-08 17:35 ` Randy Dunlap
@ 2019-12-08 19:01 ` kbuild test robot
  2019-12-10  5:07 ` Jens Axboe
  2 siblings, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2019-12-08 19:01 UTC (permalink / raw)
  To: Moritz Müller
  Cc: kbuild-all, linux-kernel, linux-block, linux-kernel,
	Moritz Müller, Philip K .

[-- Attachment #1: Type: text/plain, Size: 6787 bytes --]

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 <lkp@intel.com>

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/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 13204 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
  2019-12-08 16:59 [PATCH] floppy: hide invalid floppy disk types Moritz Müller
  2019-12-08 17:35 ` Randy Dunlap
  2019-12-08 19:01 ` kbuild test robot
@ 2019-12-10  5:07 ` Jens Axboe
  2 siblings, 0 replies; 9+ messages in thread
From: Jens Axboe @ 2019-12-10  5:07 UTC (permalink / raw)
  To: Moritz Müller, linux-kernel, linux-block, linux-kernel; +Cc: Philip K .

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. <philip@warpmail.net>
> Signed-off-by: Philip K. <philip@warpmail.net>
> Signed-off-by: Moritz Müller <moritzm.mueller@posteo.de>
> ---
>  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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
  2019-12-09 17:04     ` Denis Efremov
@ 2019-12-09 17:30       ` Philip K.
  0 siblings, 0 replies; 9+ messages in thread
From: Philip K. @ 2019-12-09 17:30 UTC (permalink / raw)
  To: Denis Efremov; +Cc: moritzm.mueller, linux-kernel, linux-block, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1510 bytes --]


> Hmm, I would say that driver blacklisting is a more proper solution in
> this case. I doubt there are people with this issue and real floppy drives
> in their setup. Altering the default driver's initialization scheme seems
> superfluous to me.

As long as major distributions like Ubuntu ship the floppy module, there
are enough people who could be affected by this peculiar behaviour.
While I agree that blacklisting the module would be more elegant, I
still think that a patch that goes in this direction could help more
people, especially those who don't want or cannot solve kernel-related
issues.

> This will force users (if there are ones) who depends on this behavior
> to rebuild the kernel. blacklisting doesn't require kernel rebuild.

Are there floppy disks of unknown types? Our patch is intentionally
conservative: We won't hide false negatives. If the motherboard reports
an non-existent disk

If you're ready to think about it, we could consider extending the patch
to un-register the device if it can recognise that it's (probably) not
real. In our case, for example, fdisk reported that fd0 had a size of
4k, something think is a strong indicator that something's not right.

Alternatively, we could look into what the comment

	/* FIXME: additional physical CMOS drive detection should go here */

would imply. This particular bug can only affect fd0 and fd1, so if we
spent some more time, we could find something.

-- 
	With kind regards,
	Philip K.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
       [not found]   ` <87h82ajzqd.fsf@bulbul>
@ 2019-12-09 17:04     ` Denis Efremov
  2019-12-09 17:30       ` Philip K.
  0 siblings, 1 reply; 9+ messages in thread
From: Denis Efremov @ 2019-12-09 17:04 UTC (permalink / raw)
  To: Philip K.; +Cc: moritzm.mueller, linux-kernel, linux-block, linux-kernel

On 12/9/19 12:04 AM, Philip K. wrote:
> Denis Efremov <yefremov.denis@gmail.com> writes:
> 
>> Hi,
>>
>> On 08.12.2019 22:45, Moritz Müller wrote:
>>> In some cases floppy disks are being indexed, even though no actual
>>> device exists. In our case this was caused by the CMOS-RAM having a few
>>> peculiar bits. This caused a non-existent floppy disk of the type 13 to
>>> be registered as an possibly mountable device, even though it could not
>>> be mounted by any user.
>>>
>>> We believe this to be an instance of this bug, as we had similar logs
>>> and issues:
>>>
>>>  https://bugzilla.kernel.org/show_bug.cgi?id=13486
>>>  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/384579

Well, this is a ten years old bug. It seems like that time it was decided
to fix the problem either by blacklisting the driver or by disabling the
BIOS option. I doubt that many people are facing this problem since then.
I think that more-or-less modern motherboards don't have this issue.

>>>
>>> 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.
>>
>> Does driver blacklisting solves your problem? Or you have real floppy drives in
>> your system along with these "spurious" ones?
> 
> No there were not, and blacklisting would solve the bug too. We just
> thought that fixing the bug this way would prevent the issue from
> appearing in the first place on systems that have the floppy module
> enabled, in the first place.

Hmm, I would say that driver blacklisting is a more proper solution in
this case. I doubt there are people with this issue and real floppy drives
in their setup. Altering the default driver's initialization scheme seems
superfluous to me. This will force users (if there are ones) who depends on this
behavior to rebuild the kernel. blacklisting doesn't require kernel rebuild.

> 
>>> Co-developed-by: Philip K. <philip@warpmail.net>
>>> Signed-off-by: Philip K. <philip@warpmail.net>
>>> Signed-off-by: Moritz Müller <moritzm.mueller@posteo.de>
>>> ---
>>>  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 recognizes 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;
>>> +#endif
>>
>> You can't just skip it with ifdef. This will result in uninitialized
>> pointer dereference down the code.
>>
>> 		struct floppy_drive_params *params;
>> 		...
>>
>> 		if (type < ARRAY_SIZE(default_drive_params)) {
>> 			...
>> 		} else {
>> #ifdef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
>> 			params = &default_drive_params[0].params;
>> #endif
>> 			...
>> 		}
>> 		...
>> 		*UDP = *params; // << HERE
> 
> Oops, you're right, will fix.
> 
>>>  			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;
>>>  }
>>>  
>>>
>>
>> Thanks,
>> Denis
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
  2019-12-08 19:45 Moritz Müller
  2019-12-08 20:16 ` Denis Efremov
@ 2019-12-09  0:32 ` kbuild test robot
  1 sibling, 0 replies; 9+ messages in thread
From: kbuild test robot @ 2019-12-09  0:32 UTC (permalink / raw)
  To: Moritz Müller
  Cc: kbuild-all, linux-kernel, linux-block, linux-kernel,
	Moritz Müller, Philip K .

[-- Attachment #1: Type: text/plain, Size: 1895 bytes --]

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-035056
base:   https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git for-next
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.5.0-1) 7.5.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   drivers//block/floppy.c: In function 'floppy_available':
>> drivers//block/floppy.c:4524:6: error: 'type' undeclared (first use in this function); did you mean 'true'?
     if (type >= ARRAY_SIZE(default_drive_params))
         ^~~~
         true
   drivers//block/floppy.c:4524:6: note: each undeclared identifier is reported only once for each function it appears in

vim +4524 drivers//block/floppy.c

  4516	
  4517	static bool floppy_available(int drive)
  4518	{
  4519		if (!(allowed_drive_mask & (1 << drive)))
  4520			return false;
  4521		if (fdc_state[FDC(drive)].version == FDC_NONE)
  4522			return false;
  4523	#ifndef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
> 4524		if (type >= ARRAY_SIZE(default_drive_params))
  4525			return false;
  4526	#endif
  4527		return true;
  4528	}
  4529	

---
0-DAY kernel test infrastructure                 Open Source Technology Center
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 44037 bytes --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] floppy: hide invalid floppy disk types
  2019-12-08 19:45 Moritz Müller
@ 2019-12-08 20:16 ` Denis Efremov
       [not found]   ` <87h82ajzqd.fsf@bulbul>
  2019-12-09  0:32 ` kbuild test robot
  1 sibling, 1 reply; 9+ messages in thread
From: Denis Efremov @ 2019-12-08 20:16 UTC (permalink / raw)
  To: Moritz Müller, linux-kernel, linux-block, linux-kernel; +Cc: Philip K .

Hi,

On 08.12.2019 22:45, Moritz Müller wrote:
> In some cases floppy disks are being indexed, even though no actual
> device exists. In our case this was caused by the CMOS-RAM having a few
> peculiar bits. This caused a non-existent floppy disk of the type 13 to
> be registered as an possibly mountable device, even though it could not
> be mounted by any user.
> 
> We believe this to be an instance of this bug, as we had similar logs
> and issues:
> 
>  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.

Does driver blacklisting solves your problem? Or you have real floppy drives in
your system along with these "spurious" ones?

> 
> Co-developed-by: Philip K. <philip@warpmail.net>
> Signed-off-by: Philip K. <philip@warpmail.net>
> Signed-off-by: Moritz Müller <moritzm.mueller@posteo.de>
> ---
>  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 recognizes 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;
> +#endif

You can't just skip it with ifdef. This will result in uninitialized
pointer dereference down the code.

		struct floppy_drive_params *params;
		...

		if (type < ARRAY_SIZE(default_drive_params)) {
			...
		} else {
#ifdef CONFIG_FLOPPY_ALLOW_UNKNOWN_TYPES
			params = &default_drive_params[0].params;
#endif
			...
		}
		...
		*UDP = *params; // << HERE

>  			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;
>  }
>  
> 

Thanks,
Denis

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] floppy: hide invalid floppy disk types
@ 2019-12-08 19:45 Moritz Müller
  2019-12-08 20:16 ` Denis Efremov
  2019-12-09  0:32 ` kbuild test robot
  0 siblings, 2 replies; 9+ messages in thread
From: Moritz Müller @ 2019-12-08 19:45 UTC (permalink / raw)
  To: linux-kernel, linux-block, linux-kernel; +Cc: Moritz Müller, Philip K .

In some cases floppy disks are being indexed, even though no actual
device exists. In our case this was caused by the CMOS-RAM having a few
peculiar bits. This caused a non-existent floppy disk of the type 13 to
be registered as an possibly mountable device, even though it could not
be mounted by any user.

We believe this to be an instance of this bug, as we had similar logs
and issues:

 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. <philip@warpmail.net>
Signed-off-by: Philip K. <philip@warpmail.net>
Signed-off-by: Moritz Müller <moritzm.mueller@posteo.de>
---
 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 recognizes 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;
+#endif
 			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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2019-12-10  5:07 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-08 16:59 [PATCH] floppy: hide invalid floppy disk types Moritz Müller
2019-12-08 17:35 ` Randy Dunlap
2019-12-08 19:01 ` kbuild test robot
2019-12-10  5:07 ` Jens Axboe
2019-12-08 19:45 Moritz Müller
2019-12-08 20:16 ` Denis Efremov
     [not found]   ` <87h82ajzqd.fsf@bulbul>
2019-12-09 17:04     ` Denis Efremov
2019-12-09 17:30       ` Philip K.
2019-12-09  0:32 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).