Re: [PATCH net-next] tcp: fix incorrect parameter validation in the do_tcp_getsockopt() function

[Gavrilov Ilia]

С уважением,
Илья Гаврилов
Ведущий программист
Отдел разработки
АО "ИнфоТеКС" в г. Санкт-Петербург
127287, г. Москва, Старый Петровско-Разумовский проезд, дом 1/23, стр. 1
T: +7 495 737-61-92 ( доб. 4921)
Ф: +7 495 737-72-78


Ilia.Gavrilov@xxxxxxxxxxx
www.infotecs.ru


On 3/7/24 11:40, Simon Horman wrote:
> On Wed, Mar 06, 2024 at 11:54:40AM +0000, Gavrilov Ilia wrote:
>> On 3/6/24 14:36, Paolo Abeni wrote:
>>> The above is incorrect, as the 'len' variable is a signed integer
>>
>> I mean, if 'len' is negative then after this expression
>> len = min_t(unsigned int, len, sizeof(int));
>> the 'len' variable will be equal to sizeof(int) == 4
>> and the statement
>> if (len < 0) return -EINVAL;
>> might be unreachable during program execution.
>
> Hi Gavrilov and Paolo,
>
> I could be missing something obvious but it seems to me that this is correct.
> Although perhaps we could try rewording the patch description to
> make things a bit clearer. Here is my attempt at that:
>
>     The 'len' variable can't be negative when assigned the result of
>     'min_t' because all 'min_t' parameters are cast to unsigned int,
>     and then the minimum one is chosen.
>
>     To fix the logic, check 'len' as read from 'optlen',
>     where the types of relevant variables are (signed) int.
>
> FWIIW, I see four similar patches on netdev this morning.
> It does seem to me that they are all valid fixes.
> But if they need to be reposted, or there are more coming,
> then I think it would be useful to bundle them up,
> say into batches of 10, and send as patch-sets.
>
> This may help with fragmentation of review of what seems
> to be the same change in multiple places.
>
>

Hi Simon, thank you for your answer.

I'll reword the patch description and repost a series of patches in V2.
I also found a couple of places with the same problem.