netdev - Re: [PATCH net-next] tipc: potential memory corruption in tipc_crypto_key

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: <20200923.180615.357823799741297427.davem@davemloft.net>
Date:   Wed, 23 Sep 2020 18:06:15 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     dan.carpenter@...cle.com
Cc:     jmaloy@...hat.com, ying.xue@...driver.com, kuba@...nel.org,
        tuong.t.lien@...tech.com.au, netdev@...r.kernel.org,
        tipc-discussion@...ts.sourceforge.net,
        kernel-janitors@...r.kernel.org, Julia.Lawall@...6.fr,
        keescook@...omium.org
Subject: Re: [PATCH net-next] tipc: potential memory corruption in
 tipc_crypto_key_rcv()

From: Dan Carpenter <dan.carpenter@...cle.com>
Date: Wed, 23 Sep 2020 11:30:17 +0300

> This code uses "skey->keylen" as an memcpy() size and then checks that
> it is valid on the next line.  The other problem is that the check has
> a potential integer overflow, it's better to use struct_size() for this.
> 
> Fixes: 23700da29b83 ("tipc: add automatic rekeying for encryption key")
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> ---
> Hey Kees and Julia,
> 
> It would be nice to change tipc_aead_key_size() but I'm not sure how the
> UAPI stuff works.  My first attempt at to change it to
> 
> 	return struct_size(key, key, key->keylen);
> 
> broke the build.  I think you guys used Coccinelle to automatically
> update these calculations.  Probably this wasn't updated because you
> didn't want to break the build either?

If it is subject to overflows, the tipc_aead_key_size() helper
shouldn't be used as-is by userspace either.

Right?

Please find a way to fix that inline function instead without breaking
UAPI, thank you.