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  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]
Date:   Mon, 12 Dec 2022 13:10:26 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        Jacob Keller <jacob.e.keller@...el.com>,
        linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH net] genetlink: Fix an error handling path in
 ctrl_dumppolicy_start()

On Mon, 12 Dec 2022 22:03:06 +0100 Christophe JAILLET wrote:
> If this memory allocation fails, some resources need to be freed.
> Add the missing goto to the error handling path.
> 
> Fixes: b502b3185cd6 ("genetlink: use iterator in the op to policy map dumping")
> Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> ---
> This patch is speculative.
> 
> This function is a callback and I don't know how the core works and handles
> such situation, so review with care!

It's fine, the function has pretty much two completely separate paths.
Dump all ops and dump a single op.
Anything that allocs state before this point is on the single op path,
while the iterator is only allocated for dump all.
This should be evident from the return 0; at the end of the 
  if (tb[CTRL_ATTR_OP])

> More-over, should this kmalloc() be a kzalloc()?
> genl_op_iter_init() below does not initialize all fields, be they are maybe
> set correctly before uses.

It's fine, op_iters are put on the stack without initializing, iter
init must (and currently does) work without depending on zeroed memory.

Powered by blists - more mailing lists