netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jakub Kicinski <kuba@kernel.org>
To: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Jacob Keller <jacob.e.keller@intel.com>,
	linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org,
	netdev@vger.kernel.org
Subject: Re: [PATCH net] genetlink: Fix an error handling path in ctrl_dumppolicy_start()
Date: Mon, 12 Dec 2022 13:10:26 -0800	[thread overview]
Message-ID: <20221212131026.120bdd71@kernel.org> (raw)
In-Reply-To: <7186dae6d951495f6918c45f8250e6407d71e88f.1670878949.git.christophe.jaillet@wanadoo.fr>

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@wanadoo.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.

  reply	other threads:[~2022-12-12 21:10 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-12 21:03 [PATCH net] genetlink: Fix an error handling path in ctrl_dumppolicy_start() Christophe JAILLET
2022-12-12 21:10 ` Jakub Kicinski [this message]
2022-12-12 21:23   ` Keller, Jacob E

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221212131026.120bdd71@kernel.org \
    --to=kuba@kernel.org \
    --cc=christophe.jaillet@wanadoo.fr \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=jacob.e.keller@intel.com \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).