netdev - Re: [PATCH iproute2-next 1/2] Add support for IOAM encap modes

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] [day] [month] [year] [list]

Message-ID: <1623524265.114537131.1633445805175.JavaMail.zimbra@uliege.be>
Date:   Tue, 5 Oct 2021 16:56:45 +0200 (CEST)
From:   Justin Iurman <justin.iurman@...ege.be>
To:     David Ahern <dsahern@...il.com>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, dsahern@...nel.org,
        stephen@...workplumber.org
Subject: Re: [PATCH iproute2-next 1/2] Add support for IOAM encap modes

>>>> +static const char *ioam6_mode_types[] = {
>>>
>>> I think you want to declare this of size IOAM6_IPTUNNEL_MODE_MAX + 1
>> 
>> This is automatically the case, see below explanation.
>> 
>>>> +	[IOAM6_IPTUNNEL_MODE_INLINE]	= "inline",
>>>> +	[IOAM6_IPTUNNEL_MODE_ENCAP]	= "encap",
>>>> +	[IOAM6_IPTUNNEL_MODE_AUTO]	= "auto",
>>>> +};
>>>> +
>>>> +static const char *format_ioam6mode_type(int mode)
>>>> +{
>>>> +	if (mode < IOAM6_IPTUNNEL_MODE_MIN ||
>>>> +	    mode > IOAM6_IPTUNNEL_MODE_MAX ||
>>>> +	    !ioam6_mode_types[mode])
>>>
>>> otherwise this check is not sufficient.
>> 
>> Are you sure? I mean, both IOAM6_IPTUNNEL_MODE_MIN and IOAM6_IPTUNNEL_MODE_MAX
>> respectively point to IOAM6_IPTUNNEL_MODE_INLINE and IOAM6_IPTUNNEL_MODE_AUTO.
>> So, either the input mode is out of bound, or not defined in the array above
>> (this one is not mandatory, but it ensures that the above array is updated
>> accordingly with the uapi). So, what we have right now is:
>> 
>> __IOAM6_IPTUNNEL_MODE_MIN = 0
>> IOAM6_IPTUNNEL_MODE_INLINE = 1
>> IOAM6_IPTUNNEL_MODE_ENCAP = 2
>> IOAM6_IPTUNNEL_MODE_AUTO = 3
>> __IOAM6_IPTUNNEL_MODE_MAX = 4
>> 
>> IOAM6_IPTUNNEL_MODE_MIN = 1
>> IOAM6_IPTUNNEL_MODE_MAX = 3
>> 
>> ioam6_mode_types = {
>>   [0] (null)
>>   [1] "inline"
>>   [2] "encap"
>>   [3] "auto"
>> }
>> 
>> where its size is automatically/implicitly 4 (IOAM6_IPTUNNEL_MODE_MAX + 1).
>> 
> 
> today yes, but tomorrow no. ie,. a new feature is added to the header
> file. Header file is updated in iproute2 as part of a header file sync
> but the ioam6 code is not updated to expand ioam6_mode_types. Command is
> then run on a system with the new feature so
> 
>    mode > IOAM6_IPTUNNEL_MODE_MAX
> 
> will pass but then
> 
>     !ioam6_mode_types[mode])
> 
> accesses an entry beyond the size of ioam6_mode_types.

Indeed, sorry. I'll send the fix in -v2.

Thanks!