netdev - Re: [PATCH] ionic: fix 16bit math issue when PAGE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 14 Sep 2023 13:28:34 -0700
From: "Nelson, Shannon" <shannon.nelson@....com>
To: David Christensen <drc@...ux.vnet.ibm.com>, brett.creeley@....com,
 drivers@...sando.io
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH] ionic: fix 16bit math issue when PAGE_SIZE >= 64KB

On 9/12/2023 3:31 PM, David Christensen wrote:
> 
> On 9/11/23 5:24 PM, Nelson, Shannon wrote:
> 
>>> @@ -452,7 +452,7 @@ void ionic_rx_fill(struct ionic_queue *q)
>>>
>>>                  /* fill main descriptor - buf[0] */
>>>                  desc->addr = cpu_to_le64(buf_info->dma_addr +
>>> buf_info->page_offset);
>>> -               frag_len = min_t(u16, len, IONIC_PAGE_SIZE -
>>> buf_info->page_offset);
>>> +               frag_len = min_t(u32, len, IONIC_PAGE_SIZE -
>>> buf_info->page_offset);
>>>                  desc->len = cpu_to_le16(frag_len);
>>
>> Hmm... using cpu_to_le16() on a 32-bit value looks suspect - it might
>> get forced to 16-bit, but looks funky, and might not be as successful in
>> a BigEndian environment.
>>
>> Since the descriptor and sg_elem length fields are limited to 16-bit,
>> there might need to have something that assures that the resulting
>> lengths are never bigger than 64k - 1.
>>
> 
> What do you think about this:
> 
>   frag_len = min_t(u16, len, min_t(u32, 0xFFFF, IONIC_PAGE_SIZE -
> buf_info->page_offset));

Yes, that looks a little safer.

We'll still need to do something about the 32-bit frag_len used in the 
cpu_to_le16() call.

> 
> Can you think of a test case where buf_info->page_offset will be
> non-zero that I can test locally?

No, I don't have one off-hand.

Thanks,
sln


> 
> Dave