Re: CVS commit: src/sys/dev

[Rin Okuyama <rokuyama.rk%gmail.com@localhost>]

Hi Michael,

On 2025/04/12 21:00, Michael van Elst wrote:
> On Sat, Apr 12, 2025 at 08:14:28PM +0900, Rin Okuyama wrote:
> > Hi Michael, thanks for kind review!
>
> Hi rin,
>
>
> > PS
> > For ld_sdmmc.c, IIUC, check for ld_sdmmc_dump() yields
> >
> > ```
> > if (blkno + blkcnt - 1 > sc->sc_sf->csd.capacity)
> > 	return EIO;
>
>
> capacity is the block count, not the last block number. So there
> you need to check
>
> if (blkno + blkcnt > sc->sc_sf->csd.capacity)
> 	return EIO;
>
> or
>
> if (blkno + blkcnt - 1 >= sc->sc_sf->csd.capacity)
> 	return EIO;

Oops, I was also careless here... Thanks again for pointing it out!

> > If so, shouldn't we take into account b_bcount for ld_sdmmc_dobio()?
> > https://nxr.netbsd.org/xref/src/sys/dev/sdmmc/ld_sdmmc.c#458
>
> Yes. It needs to take into account b_bcount.
>
> However, all I/O is already validated before it reaches the driver,
> the condition in ld_sdmmc.c will never be met.
>
> Almost all disk drivers use helper routines in sys/dev/dksubr.c.
>
> dk_translate does the validation for regular I/O by calling
> bounds_check_with_mediasize() or bounds_check_with_label().
>
> dk_dump does the validation for a dump operation directly
> against the disk_geom data.

Got it. Thank you very much for detailed explanation!
I will leave ld_sdmmc_dobio() as is for now.

Thanks,
rin

> Greetings,