vhost: Check docket sk_family instead of call getname
PATCH 1/1 vhost: Check docket sk_family instead of call getname
From: Eugenio Pérez <[email protected]>
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).
Upstream: 42d84c8490f9f0931786f1623191fcab397c3d64
--- 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.
drivers/vhost/net.c +2 -11
@@ -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; }
Signature · 6 lines
2.20.1 -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
_______________________________________________
cip-dev mailing list
[email protected]
https://lists.cip-project.org/mailman/listinfo/cip-dev
Hi Pavel,
A couple of comments below -
Quoted 10 lines · Pavel Machek
From: Eugenio Pérez <[email protected]>
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_llparameter beyond its size, we add a little bit of security here. It should do not do beyondMAX_ADDR_LEN, but syzbot found thatax25_getnamewrites more (72 bytes, the size offull_sockaddr_ax25, versus 20 + 32 bytes ofsockaddr_ll+MAX_ADDR_LENin syzbot repro).
Quoted 6 lines
Fixes 3a4d5c94e9593 ("vhost_net: a kernel-level virtio server")Reported-by [email protected]Signed-off-by Eugenio Pérez <[email protected]>Signed-off-by David S. Miller <[email protected]>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.
Quoted 44 lines
Signed-off-by Pavel Machek <[email protected]>--- 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.
drivers/vhost/net.c +2
-11@@ -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; }Signature · 1 line
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
_______________________________________________
cip-dev mailing list
[email protected]
https://lists.cip-project.org/mailman/listinfo/cip-dev