Re: [PATCH V2] block: fix get_max_segment_size() overflow on 32bit arch

Ming Lei
2020-01-12T08:00:19Z

On Sat, Jan 11, 2020 at 08:51:31AM -0700, Jens Axboe wrote:
> On 1/11/20 5:57 AM, Ming Lei wrote:
> > Commit 429120f3df2d starts to take account of segment's start dma address
> > when computing max segment size, and data type of 'unsigned long'
> > is used to do that. However, the segment mask may be 0xffffffff, so
> > the figured out segment size may be overflowed in case of zero physical
> > address on 32bit arch.
> >
> > Fix the issue by returning queue_max_segment_size() directly when that
> > happens.
>
> I still think this should use phys_addr_t, just in case the mask is
> ever not 32-bit. The current types are a bit weird, tbh.

I didn't use phys_addr_t because queue_segment_boundary() always
returns 'unsigned long', so using 'phys_addr_t' doesn't make any
difference because the following result can be held in 32bit always
no matter offset is 32bit or 64bit:

mask & (page_to_phys(start_page) + offset)

BTW, 'seg_boundary_mask' is defined as 'unsigned long' since kernel
git tree was born. Given not see related report with 64bit phys_addr_t
on 32bit arch, I guess we may leave it alone.

However, if you think we need to convert 'seg_boundary_mask' into
phys_addr_t, the 'offset' parameter can be re-defined as phys_addr_t.

thanks,
Ming