netdev - Re: [PATCH v1 1/1] mac_pton: Don't access memory over expected length

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: <20221010173809.5f863ea6@kernel.org>
Date:   Mon, 10 Oct 2022 17:38:09 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Andrew Lunn <andrew@...n.ch>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [PATCH v1 1/1] mac_pton: Don't access memory over expected
 length

On Mon, 10 Oct 2022 22:19:49 +0200 Andrew Lunn wrote:
> On Wed, Oct 05, 2022 at 07:43:01PM +0300, Andy Shevchenko wrote:
> > The strlen() may go too far when estimating the length of
> > the given string. In some cases it may go over the boundary
> > and crash the system which is the case according to the commit
> > 13a55372b64e ("ARM: orion5x: Revert commit 4904dbda41c8.").
> > 
> > Rectify this by switching to strnlen() for the expected
> > maximum length of the string.  
> 
> This seems like something which should have a fixes: tag, and be
> against net, not net-next.

Quoting DaveM's revert mentioned in the commit message:

    First of all, the orion5x buffer is not NULL terminated.  mac_pton()
    has no business operating on non-NULL terminated buffers because
    only the caller can know that this is valid and in what manner it
    is ok to parse this NULL'less buffer.
    
    Second of all, orion5x operates on an __iomem pointer, which cannot
    be dereferenced using normal C pointer operations.  Accesses to
    such areas much be performed with the proper iomem accessors.

So AFAICT only null-terminated strings are expected here, this patch
does not fix any known issue. No need to put it in net (if it's needed
at all).