Message ID | 20230208205642.270567-8-iii@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | BPF |
Headers | show |
Series | selftests/bpf: Add Memory Sanitizer support | expand |
On Wed, Feb 8, 2023 at 12:57 PM Ilya Leoshkevich <iii@linux.ibm.com> wrote: > > The code assumes that everything that comes after nlmsgerr are nlattrs. > When calculating their size, it does not account for the initial > nlmsghdr. This may lead to accessing uninitialized memory. > > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> > --- > tools/lib/bpf/nlattr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/nlattr.c b/tools/lib/bpf/nlattr.c > index 3900d052ed19..c5da7662bb04 100644 > --- a/tools/lib/bpf/nlattr.c > +++ b/tools/lib/bpf/nlattr.c > @@ -178,7 +178,7 @@ int libbpf_nla_dump_errormsg(struct nlmsghdr *nlh) > hlen += nlmsg_len(&err->msg); > > attr = (struct nlattr *) ((void *) err + hlen); > - alen = nlh->nlmsg_len - hlen; > + alen = (char *)nlh + nlh->nlmsg_len - (char *)attr; we use (void *) for pointer manipulations, let's be consistent? Otherwise looks good (I think, this whole nlattr stuff is very cryptic to me). > > if (libbpf_nla_parse(tb, NLMSGERR_ATTR_MAX, attr, alen, > extack_policy) != 0) { > -- > 2.39.1 >
diff --git a/tools/lib/bpf/nlattr.c b/tools/lib/bpf/nlattr.c index 3900d052ed19..c5da7662bb04 100644 --- a/tools/lib/bpf/nlattr.c +++ b/tools/lib/bpf/nlattr.c @@ -178,7 +178,7 @@ int libbpf_nla_dump_errormsg(struct nlmsghdr *nlh) hlen += nlmsg_len(&err->msg); attr = (struct nlattr *) ((void *) err + hlen); - alen = nlh->nlmsg_len - hlen; + alen = (char *)nlh + nlh->nlmsg_len - (char *)attr; if (libbpf_nla_parse(tb, NLMSGERR_ATTR_MAX, attr, alen, extack_policy) != 0) {
The code assumes that everything that comes after nlmsgerr are nlattrs. When calculating their size, it does not account for the initial nlmsghdr. This may lead to accessing uninitialized memory. Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com> --- tools/lib/bpf/nlattr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)