diff mbox series

[RESEND,v3] nbd_genl_status: null check for nla_nest_start

Message ID 20230323193032.28483-1-mkoutny@suse.com
State New
Headers
Series [RESEND,v3] nbd_genl_status: null check for nla_nest_start |

Commit Message

Michal Koutný March 23, 2023, 7:30 p.m. UTC 
  From: Navid Emamdoost <navid.emamdoost@gmail.com>

nla_nest_start may fail and return NULL. The check is inserted, and
errno is selected based on other call sites within the same source code.
Update: removed extra new line.
v3 Update: added release reply, thanks to Michal Kubecek for pointing
out.

Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
Link: https://lore.kernel.org/r/20190911164013.27364-1-navid.emamdoost@gmail.com/
---

I'm resending the patch because there was apparent consensus of its
inclusion and it seems it was only overlooked. Some people may care
about this because of CVE-2019-16089.

 drivers/block/nbd.c | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Jens Axboe March 23, 2023, 10:51 p.m. UTC | #1 
On 3/23/23 1:30 PM, Michal Koutný wrote:
> From: Navid Emamdoost <navid.emamdoost@gmail.com>
> 
> nla_nest_start may fail and return NULL. The check is inserted, and
> errno is selected based on other call sites within the same source code.
> Update: removed extra new line.
> v3 Update: added release reply, thanks to Michal Kubecek for pointing
> out.

Josef? Looks straight forward to me, though it's not clear (to me) how
this can be triggered and hence how important it is.

> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> Reviewed-by: Michal Kubecek <mkubecek@suse.cz>
> Link: https://lore.kernel.org/r/20190911164013.27364-1-navid.emamdoost@gmail.com/
> ---
> 
> I'm resending the patch because there was apparent consensus of its
> inclusion and it seems it was only overlooked. Some people may care
> about this because of CVE-2019-16089.

Anyone can file a CVE, and in fact they are often filed as some kind
of silly trophy. Whether a CVE exists or not has ZERO bearing on
whether a bug is worth fixing.

So please don't mix CVEs into any of this, they don't matter one bit.
Never have, and never will. What's important is how the bug can be
triggered.
Michal Koutný March 24, 2023, 10:16 a.m. UTC | #2 
Thanks for the reply.

On Thu, Mar 23, 2023 at 04:51:17PM -0600, Jens Axboe <axboe@kernel.dk> wrote:
> So please don't mix CVEs into any of this, they don't matter one bit.

Do not shoot the messenger.

(But I'll refrain from that numeric reference to disincentivize such
trophy collecting.)

> Never have, and never will. What's important is how the bug can be
> triggered.

From my perspective it's pragmatic better-safe-than-sorry -- a proof may
be conceived that rules out any triggering condition, it's less work to
put the guard in though.

My .02€,
Michal
diff mbox series

Patch

diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 592cfa8b765a..109dccd9a515 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2394,6 +2394,12 @@  static int nbd_genl_status(struct sk_buff *skb, struct genl_info *info)
 	}
 
 	dev_list = nla_nest_start_noflag(reply, NBD_ATTR_DEVICE_LIST);
+	if (!dev_list) {
+		nlmsg_free(reply);
+		ret = -EMSGSIZE;
+		goto out;
+	}
+
 	if (index == -1) {
 		ret = idr_for_each(&nbd_index_idr, &status_cb, reply);
 		if (ret) {