| Message ID | 1555066547-9656-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp |
|---|---|
| State | Accepted |
| Delegated to: | David Miller |
| Headers | show |
| Series | [1/9] net/rds: Check address length before reading address family | expand |
On Fri, Apr 12, 2019 at 3:56 AM Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote: > > KMSAN will complain if valid address length passed to bpf_bind() is > shorter than sizeof("struct sockaddr"->sa_family) bytes. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > --- > net/core/filter.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/net/core/filter.c b/net/core/filter.c > index 41f633cf4fc1..b9089fda4367 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -4458,6 +4458,8 @@ BPF_CALL_3(bpf_bind, struct bpf_sock_addr_kern *, ctx, struct sockaddr *, addr, > * Only binding to IP is supported. > */ > err = -EINVAL; > + if (addr_len < offsetofend(struct sockaddr, sa_family)) > + return err; the verifier will check that addr_len is not zero, but it can be one byte, so it's a good check. Thanks! > if (addr->sa_family == AF_INET) { > if (addr_len < sizeof(struct sockaddr_in)) > return err; > -- > 2.16.5 >
Alexei Starovoitov <alexei.starovoitov@gmail.com> [Fri, 2019-04-12 09:20 -0700]: > On Fri, Apr 12, 2019 at 3:56 AM Tetsuo Handa > <penguin-kernel@i-love.sakura.ne.jp> wrote: > > > > KMSAN will complain if valid address length passed to bpf_bind() is > > shorter than sizeof("struct sockaddr"->sa_family) bytes. > > > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> > > --- > > net/core/filter.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/net/core/filter.c b/net/core/filter.c > > index 41f633cf4fc1..b9089fda4367 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -4458,6 +4458,8 @@ BPF_CALL_3(bpf_bind, struct bpf_sock_addr_kern *, ctx, struct sockaddr *, addr, > > * Only binding to IP is supported. > > */ > > err = -EINVAL; > > + if (addr_len < offsetofend(struct sockaddr, sa_family)) > > + return err; > > the verifier will check that addr_len is not zero, > but it can be one byte, so it's a good check. > Thanks! True, I missed this corner-case. Thanks for fixing. Acked-by: Andrey Ignatov <rdna@fb.com> > > if (addr->sa_family == AF_INET) { > > if (addr_len < sizeof(struct sockaddr_in)) > > return err; > > -- > > 2.16.5 > >
From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Date: Fri, 12 Apr 2019 19:55:47 +0900 > KMSAN will complain if valid address length passed to bpf_bind() is > shorter than sizeof("struct sockaddr"->sa_family) bytes. > > Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> Applied.
diff --git a/net/core/filter.c b/net/core/filter.c index 41f633cf4fc1..b9089fda4367 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -4458,6 +4458,8 @@ BPF_CALL_3(bpf_bind, struct bpf_sock_addr_kern *, ctx, struct sockaddr *, addr, * Only binding to IP is supported. */ err = -EINVAL; + if (addr_len < offsetofend(struct sockaddr, sa_family)) + return err; if (addr->sa_family == AF_INET) { if (addr_len < sizeof(struct sockaddr_in)) return err;
KMSAN will complain if valid address length passed to bpf_bind() is shorter than sizeof("struct sockaddr"->sa_family) bytes. Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp> --- net/core/filter.c | 2 ++ 1 file changed, 2 insertions(+)