* [PATCH] block: floppy: fix uninitialized use of outparam in fd_locked_ioctl
@ 2025-07-06 7:22 Purva Yeshi
2025-07-12 19:16 ` Denis Efremov
0 siblings, 1 reply; 3+ messages in thread
From: Purva Yeshi @ 2025-07-06 7:22 UTC (permalink / raw)
To: efremov, axboe; +Cc: linux-block, linux-kernel, Purva Yeshi
Fix Smatch-detected error:
drivers/block/floppy.c:3569 fd_locked_ioctl() error:
uninitialized symbol 'outparam'.
Use the outparam pointer only after it is explicitly initialized.
Previously, fd_copyout() was called unconditionally after the switch-case
statement, assuming outparam would always be set when _IOC_READ was active.
However, not all paths ensured this, which led to potential use of an
uninitialized pointer.
Move fd_copyout() calls directly into the relevant case blocks immediately
after outparam is set. This ensures it is only called when safe and
applicable.
Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
---
drivers/block/floppy.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index e97432032f01..34ef756bb3b7 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -3482,6 +3482,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
memcpy(&inparam.g, outparam,
offsetof(struct floppy_struct, name));
outparam = &inparam.g;
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDMSGON:
drive_params[drive].flags |= FTD_MSG;
@@ -3515,6 +3516,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
return 0;
case FDGETMAXERRS:
outparam = &drive_params[drive].max_errors;
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDSETMAXERRS:
drive_params[drive].max_errors = inparam.max_errors;
@@ -3522,6 +3524,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
case FDGETDRVTYP:
outparam = drive_name(type, drive);
SUPBOUND(size, strlen((const char *)outparam) + 1);
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDSETDRVPRM:
if (!valid_floppy_drive_params(inparam.dp.autodetect,
@@ -3531,6 +3534,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
break;
case FDGETDRVPRM:
outparam = &drive_params[drive];
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDPOLLDRVSTAT:
if (lock_fdc(drive))
@@ -3541,17 +3545,20 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
fallthrough;
case FDGETDRVSTAT:
outparam = &drive_state[drive];
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDRESET:
return user_reset_fdc(drive, (int)param, true);
case FDGETFDCSTAT:
outparam = &fdc_state[FDC(drive)];
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDWERRORCLR:
memset(&write_errors[drive], 0, sizeof(write_errors[drive]));
return 0;
case FDWERRORGET:
outparam = &write_errors[drive];
+ return fd_copyout((void __user *)param, outparam, size);
break;
case FDRAWCMD:
return floppy_raw_cmd_ioctl(type, drive, cmd, (void __user *)param);
@@ -3565,9 +3572,6 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
return -EINVAL;
}
- if (_IOC_DIR(cmd) & _IOC_READ)
- return fd_copyout((void __user *)param, outparam, size);
-
return 0;
}
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] block: floppy: fix uninitialized use of outparam in fd_locked_ioctl
2025-07-06 7:22 [PATCH] block: floppy: fix uninitialized use of outparam in fd_locked_ioctl Purva Yeshi
@ 2025-07-12 19:16 ` Denis Efremov
2025-07-13 5:52 ` Purva Yeshi
0 siblings, 1 reply; 3+ messages in thread
From: Denis Efremov @ 2025-07-12 19:16 UTC (permalink / raw)
To: Purva Yeshi, axboe; +Cc: linux-block, linux-kernel
Hello,
Thank you for the report!
On 06/07/2025 11:22, Purva Yeshi wrote:
> Fix Smatch-detected error:
> drivers/block/floppy.c:3569 fd_locked_ioctl() error:
> uninitialized symbol 'outparam'.
>
This a false-positive diagnostic. Smatch doesn't see the dependency
between FDGET... commands and _IOC_READ.
> Use the outparam pointer only after it is explicitly initialized.
> Previously, fd_copyout() was called unconditionally after the switch-case
> statement, assuming outparam would always be set when _IOC_READ was active.
if (_IOC_DIR(cmd) & _IOC_READ)
return fd_copyout((void __user *)param, outparam, size);
and all FDGET... macro are defined as _IOR(...).
> However, not all paths ensured this, which led to potential use of an
> uninitialized pointer.
Not all paths, but commands that fall under _IOC_READ condition.
>
> Move fd_copyout() calls directly into the relevant case blocks immediately
> after outparam is set. This ensures it is only called when safe and
> applicable.
If you want to suppress this "error" you can just initialize outparam
to NULL.
>
> Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
> ---
> drivers/block/floppy.c | 10 +++++++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index e97432032f01..34ef756bb3b7 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -3482,6 +3482,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
> memcpy(&inparam.g, outparam,
> offsetof(struct floppy_struct, name));
> outparam = &inparam.g;
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDMSGON:
> drive_params[drive].flags |= FTD_MSG;
> @@ -3515,6 +3516,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
> return 0;
> case FDGETMAXERRS:
> outparam = &drive_params[drive].max_errors;
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDSETMAXERRS:
> drive_params[drive].max_errors = inparam.max_errors;
> @@ -3522,6 +3524,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
> case FDGETDRVTYP:
> outparam = drive_name(type, drive);
> SUPBOUND(size, strlen((const char *)outparam) + 1);
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDSETDRVPRM:
> if (!valid_floppy_drive_params(inparam.dp.autodetect,
> @@ -3531,6 +3534,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
> break;
> case FDGETDRVPRM:
> outparam = &drive_params[drive];
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDPOLLDRVSTAT:
> if (lock_fdc(drive))
> @@ -3541,17 +3545,20 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
> fallthrough;
> case FDGETDRVSTAT:
> outparam = &drive_state[drive];
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDRESET:
> return user_reset_fdc(drive, (int)param, true);
> case FDGETFDCSTAT:
> outparam = &fdc_state[FDC(drive)];
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDWERRORCLR:
> memset(&write_errors[drive], 0, sizeof(write_errors[drive]));
> return 0;
> case FDWERRORGET:
> outparam = &write_errors[drive];
> + return fd_copyout((void __user *)param, outparam, size);
> break;
> case FDRAWCMD:
> return floppy_raw_cmd_ioctl(type, drive, cmd, (void __user *)param);
> @@ -3565,9 +3572,6 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
> return -EINVAL;
> }
>
> - if (_IOC_DIR(cmd) & _IOC_READ)
> - return fd_copyout((void __user *)param, outparam, size);
> -
> return 0;
> }
>
Thanks,
Denis
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] block: floppy: fix uninitialized use of outparam in fd_locked_ioctl
2025-07-12 19:16 ` Denis Efremov
@ 2025-07-13 5:52 ` Purva Yeshi
0 siblings, 0 replies; 3+ messages in thread
From: Purva Yeshi @ 2025-07-13 5:52 UTC (permalink / raw)
To: efremov, axboe; +Cc: linux-block, linux-kernel
On 13/07/25 00:46, Denis Efremov wrote:
> Hello,
>
> Thank you for the report!
>
> On 06/07/2025 11:22, Purva Yeshi wrote:
>> Fix Smatch-detected error:
>> drivers/block/floppy.c:3569 fd_locked_ioctl() error:
>> uninitialized symbol 'outparam'.
>>
>
> This a false-positive diagnostic. Smatch doesn't see the dependency
> between FDGET... commands and _IOC_READ.
Hi Denis,
Thank you for the detailed explanation and for reviewing patch.
>
>> Use the outparam pointer only after it is explicitly initialized.
>> Previously, fd_copyout() was called unconditionally after the switch-case
>> statement, assuming outparam would always be set when _IOC_READ was active.
>
> if (_IOC_DIR(cmd) & _IOC_READ)
> return fd_copyout((void __user *)param, outparam, size);
>
> and all FDGET... macro are defined as _IOR(...).
Yes, I see it now.
All FDGET... commands that would enter the _IOC_READ block do initialize
`outparam`, so it's guaranteed to be safe.
>
>> However, not all paths ensured this, which led to potential use of an
>> uninitialized pointer.
>
> Not all paths, but commands that fall under _IOC_READ condition.
Got it.
>
>>
>> Move fd_copyout() calls directly into the relevant case blocks immediately
>> after outparam is set. This ensures it is only called when safe and
>> applicable.
>
> If you want to suppress this "error" you can just initialize outparam
> to NULL.
I’ll update the patch to just initialize `outparam` to NULL to suppress
the warning.
I’ll send a v2 patch soon.
>
>>
>> Signed-off-by: Purva Yeshi <purvayeshi550@gmail.com>
>> ---
>> drivers/block/floppy.c | 10 +++++++---
>> 1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
>> index e97432032f01..34ef756bb3b7 100644
>> --- a/drivers/block/floppy.c
>> +++ b/drivers/block/floppy.c
>> @@ -3482,6 +3482,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
>> memcpy(&inparam.g, outparam,
>> offsetof(struct floppy_struct, name));
>> outparam = &inparam.g;
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDMSGON:
>> drive_params[drive].flags |= FTD_MSG;
>> @@ -3515,6 +3516,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
>> return 0;
>> case FDGETMAXERRS:
>> outparam = &drive_params[drive].max_errors;
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDSETMAXERRS:
>> drive_params[drive].max_errors = inparam.max_errors;
>> @@ -3522,6 +3524,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
>> case FDGETDRVTYP:
>> outparam = drive_name(type, drive);
>> SUPBOUND(size, strlen((const char *)outparam) + 1);
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDSETDRVPRM:
>> if (!valid_floppy_drive_params(inparam.dp.autodetect,
>> @@ -3531,6 +3534,7 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
>> break;
>> case FDGETDRVPRM:
>> outparam = &drive_params[drive];
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDPOLLDRVSTAT:
>> if (lock_fdc(drive))
>> @@ -3541,17 +3545,20 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
>> fallthrough;
>> case FDGETDRVSTAT:
>> outparam = &drive_state[drive];
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDRESET:
>> return user_reset_fdc(drive, (int)param, true);
>> case FDGETFDCSTAT:
>> outparam = &fdc_state[FDC(drive)];
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDWERRORCLR:
>> memset(&write_errors[drive], 0, sizeof(write_errors[drive]));
>> return 0;
>> case FDWERRORGET:
>> outparam = &write_errors[drive];
>> + return fd_copyout((void __user *)param, outparam, size);
>> break;
>> case FDRAWCMD:
>> return floppy_raw_cmd_ioctl(type, drive, cmd, (void __user *)param);
>> @@ -3565,9 +3572,6 @@ static int fd_locked_ioctl(struct block_device *bdev, blk_mode_t mode,
>> return -EINVAL;
>> }
>>
>> - if (_IOC_DIR(cmd) & _IOC_READ)
>> - return fd_copyout((void __user *)param, outparam, size);
>> -
>> return 0;
>> }
>>
>
> Thanks,
> Denis
Best regards,
Purva Yeshi
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-13 5:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-07-06 7:22 [PATCH] block: floppy: fix uninitialized use of outparam in fd_locked_ioctl Purva Yeshi
2025-07-12 19:16 ` Denis Efremov
2025-07-13 5:52 ` Purva Yeshi
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®