| Message ID | 20200327075720.GA32000@duo.ucw.cz (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [4.4] vhost: Check docket sk_family instead of call getname | expand |
Hi Pavel, A couple of comments below - Pavel Machek <pavel@ucw.cz> writes: > From: Eugenio Pérez <eperezma@redhat.com> > > Doing so, we save one call to get data we already have in the struct. > > Also, since there is no guarantee that getname use sockaddr_ll > parameter beyond its size, we add a little bit of security here. > It should do not do beyond MAX_ADDR_LEN, but syzbot found that > ax25_getname writes more (72 bytes, the size of full_sockaddr_ax25, > versus 20 + 32 bytes of sockaddr_ll + MAX_ADDR_LEN in syzbot repro). > > Fixes: 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server") > Reported-by: syzbot+f2a62d07a5198c819c7b@syzkaller.appspotmail.com > Signed-off-by: Eugenio Pérez <eperezma@redhat.com> > Acked-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: David S. Miller <davem@davemloft.net> > Upstream: 42d84c8490f9f0931786f1623191fcab397c3d64 Please follow the style guidelines documented in Documentation/process/stable-kernel-rules.rst. In this instance, please mention the upstream commit id following - The upstream commit ID must be specified with a separate line above the commit text, like this: commit <sha1> upstream. For an example, take a look at Commit ad598a48fe61 ("vhost: Check docket sk_family instead of call getname"). It's a backport of the same upstream patch to 4.19 stable kernel. > Signed-off-by: Pavel Machek <pavel@denx.de> > --- > drivers/vhost/net.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > --- > > Since it is a security problem, I backported the patch to 4.4. I > compile tested it on cip test farm, but it was only built for single > target, probably due to too big config. > > diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c > index 1459dc9fd701..5efac33c29dc 100644 > --- a/drivers/vhost/net.c > +++ b/drivers/vhost/net.c > @@ -815,11 +815,7 @@ static int vhost_net_release(struct inode *inode, struct file *f) > > static struct socket *get_raw_socket(int fd) > { > - struct { > - struct sockaddr_ll sa; > - char buf[MAX_ADDR_LEN]; > - } uaddr; > - int uaddr_len = sizeof uaddr, r; > + int r; > struct socket *sock = sockfd_lookup(fd, &r); > > if (!sock) > @@ -831,12 +827,7 @@ static struct socket *get_raw_socket(int fd) > goto err; > } > > - r = sock->ops->getname(sock, (struct sockaddr *)&uaddr.sa, > - &uaddr_len, 0); > - if (r) > - goto err; > - > - if (uaddr.sa.sll_family != AF_PACKET) { > + if (sock->sk->sk_family != AF_PACKET) { > r = -EPFNOSUPPORT; > goto err; > } > -- > 2.20.1 This patch fails to apply with the following error - % git am vhost.mbox Applying: vhost: Check docket sk_family instead of call getname error: corrupt patch at line 16 Patch failed at 0001 vhost: Check docket sk_family instead of call getname hint: Use 'git am --show-current-patch' to see the failed patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort". There is some encoding issue in your mail client. The same workflow works for other kernel patches. Thanks, Punit
diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index 1459dc9fd701..5efac33c29dc 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -815,11 +815,7 @@ static int vhost_net_release(struct inode *inode, struct file *f) static struct socket *get_raw_socket(int fd) { - struct { - struct sockaddr_ll sa; - char buf[MAX_ADDR_LEN]; - } uaddr; - int uaddr_len = sizeof uaddr, r; + int r; struct socket *sock = sockfd_lookup(fd, &r); if (!sock) @@ -831,12 +827,7 @@ static struct socket *get_raw_socket(int fd) goto err; } - r = sock->ops->getname(sock, (struct sockaddr *)&uaddr.sa, - &uaddr_len, 0); - if (r) - goto err; - - if (uaddr.sa.sll_family != AF_PACKET) { + if (sock->sk->sk_family != AF_PACKET) { r = -EPFNOSUPPORT; goto err; }