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

Message-ID: <86e6a547-0d9d-9720-15bc-81fb40e6cd84@gmail.com>
Date:   Tue, 5 Oct 2021 08:49:18 -0600
From:   David Ahern <dsahern@...il.com>
To:     Justin Iurman <justin.iurman@...ege.be>
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

On 10/5/21 8:45 AM, Justin Iurman wrote:
>>> +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.