Re: [PATCH RESEND] m68k: emu: Replace unbounded sprintf in nfhd_init_one

[Kees Cook]

On Wed, Mar 18, 2026 at 01:16:33AM +0100, Thorsten Blum wrote:
> Replace unbounded sprintf() with the safer snprintf().
> 
> Signed-off-by: Thorsten Blum <thorsten.blum@xxxxxxxxx>
> ---
>  arch/m68k/emu/nfblock.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/m68k/emu/nfblock.c b/arch/m68k/emu/nfblock.c
> index 93536cf2a38e..fa6256c5af22 100644
> --- a/arch/m68k/emu/nfblock.c
> +++ b/arch/m68k/emu/nfblock.c
> @@ -132,7 +132,8 @@ static int __init nfhd_init_one(int id, u32 blocks, u32 bsize)
>  	dev->disk->minors = 16;
>  	dev->disk->fops = &nfhd_ops;
>  	dev->disk->private_data = dev;
> -	sprintf(dev->disk->disk_name, "nfhd%u", dev_id);
> +	snprintf(dev->disk->disk_name, sizeof(dev->disk->disk_name), "nfhd%u",
> +		 dev_id);
>  	set_capacity(dev->disk, (sector_t)blocks * (bsize / 512));
>  	err = add_disk(dev->disk);
>  	if (err)

This one falls into a "currently impossible" category:

        for (i = NFHD_DEV_OFFSET; i < 24; i++) {
                if (nfhd_get_capacity(i, 0, &blocks, &bsize))
                        continue;
                nfhd_init_one(i, blocks, bsize);

static int __init nfhd_init_one(int id, u32 blocks, u32 bsize)
{
	...
        int dev_id = id - NFHD_DEV_OFFSET;
	...
        sprintf(dev->disk->disk_name, "nfhd%u", dev_id);

I'd almost prefer to see "id" bounds checked prior to the dev_id
calculation (in some unlikely future where "id" isn't
NFHD_DEV_OFFSET-based).

#define DISK_NAME_LEN                   32
...
        char disk_name[DISK_NAME_LEN];  /* name of major driver */

The largest the %u could get would be 10 digits, so it'll always be in
bounds.

But there's no harm in the change:

Reviewed-by: Kees Cook <kees@xxxxxxxxxx>


-- 
Kees Cook