Bug 1150004 - (CVE-2019-16089) VUL-1: CVE-2019-16089: kernel-source: missing null check for nla_nest_start
VUL-1: CVE-2019-16089: kernel-source: missing null check for nla_nest_start
Classification: Novell Products
Product: SUSE Security Incidents
Classification: Novell Products
Component: Incidents
Other Other
: P4 - Low : Normal
: ---
Assigned To: Michal Kubeček
Security Team bot
Depends on:
  Show dependency treegraph
Reported: 2019-09-09 11:27 UTC by Alexandros Toptsoglou
Modified: 2023-03-28 10:32 UTC (History)
11 users (show)

See Also:
Found By: Security Response Team
Services Priority:
Business Priority:
Blocker: ---
Marketing QA Status: ---
IT Deployment: ---


Note You need to log in before you can comment on or make changes to this bug.
Description Alexandros Toptsoglou 2019-09-09 11:27:21 UTC

An issue was discovered in the Linux kernel through 5.2.13. nbd_genl_status in
drivers/block/nbd.c does not check the nla_nest_start_noflag return value.

Comment 3 Michal Kubeček 2019-09-10 08:18:29 UTC
Takashi is right, the only difference between nla_nest_start_noflag() and
nla_nest_start() is that the latter always set NLA_F_NESTED flag in attribute
type while the former does not (as nla_nest_start() did not before v5.2-rc1).

AFAICS, the issue was introduced by commit 47d902b90a32 ("nbd: add a status
netlink command") in v4.12-rc1. The offending commit wasn't backported into
any of our pre-4.12 branches and the issue wasn't fixed in mainline yet so
that all branches based on 4.12 and newer are affected.

However, the patch doesn't look right as it does not call nlmsg_free(reply)
on bailout so that we would have a leak if the nla_nest_start_noflag() fails.
Fortunately, it could only happen if the allocated message size is too small
which would be a bug on its own. Which, BtW, also means that the issue itself
is rather theoretical.
Comment 4 Michal Kubeček 2019-09-10 08:41:02 UTC
(In reply to Michal Kubeček from comment #3)
> Fortunately, it could only happen if the allocated message size is too small
> which would be a bug on its own.

And the message size estimate is actually incorrect. First, the use of
nla_attr_size() only works thanks to the fact that (1) first attribute in the
inner nest is u32 so that there is no padding and (2) the padding for second
attribute will be provided by subsequent call to nla_total_size() for inner
nest header. What is actually missing would be 4 bytes for outer nest header.
This probably only works thanks to the fact that genlmsg_new() eventually
calls alloc_skb() which rounds the size with SKB_DATA_ALIGN(), i.e. rounds up
to a multiple of SMP_CACHE_BYTES.

Correct message size estimate would be

        msg_size = nla_total_size(nla_total_size(sizeof(u32)) +
        msg_size *= (index == -1) ? nbd_total_devices : 1;
        msg_size = nla_total_size(msg_size);

Anyway, even with the wrong size estimate, it still wouldn't result in failure
of this particular nla_nest_start_noflag() call unless nbd_total_devices is 0
(as this nest is opened before adding any other attributes) and that would be
almost certainly saved by the SKB_DATA_ALIGN() I mentioned above.
Comment 6 Takashi Iwai 2022-05-23 14:59:06 UTC
The needed fix can be straightforward and trivial (just a missing NULL check) in cve/linux-4.12 branch.

Michal, could you cook it quickly?  Or shall I backport?
Comment 7 Takashi Iwai 2022-05-25 14:29:46 UTC
... and looking at the latest code again, the fix patch (v3) wasn't taken to the upstream by some reason.
Comment 8 Takashi Iwai 2022-08-08 14:59:34 UTC
The upstream still misses it, and I guess that it was just a misunderstanding / overlook from nbd maintainer (Josef and/or Jens), since the post was to Michal, not to them (although they were Cc'ed).

Maybe the best would be to resubmit the v3 fix patch from Michal to nbd maintainers again.

Michal, any chance to do that?
Comment 17 Michal Koutný 2023-03-24 10:23:23 UTC
Security team, I'd like to dispute this CVE record -- I want the possible patch not to bear the CVE-2019-16089 reference.

Probabilistic proof by contradiction: If the condition was exploitable, there would likely be reports of crashes, easily identified in that particular spot. It's been at least 1292 days since the "CVE" report but there were no (our products or even upstream) reports of crashes related to the missing check, i.e. it's not exploitable.