diff mbox series

vhost: fix validate_msg_fds if VHOST_USER_VRING_NOFD_MASK set.

Message ID 1573621381-3893-1-git-send-email-wangzk320@163.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: fix validate_msg_fds if VHOST_USER_VRING_NOFD_MASK set. |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

王志克 Nov. 13, 2019, 5:03 a.m. UTC 
  When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0.

Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs")
Signed-off-by: Zhike Wang <wangzk320@163.com>
---
 lib/librte_vhost/vhost_user.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

Comments

Maxime Coquelin Nov. 13, 2019, 9:53 a.m. UTC | #1 
Hi Zhike,

On 11/13/19 6:03 AM, Zhike Wang wrote:
> When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0.
> 
> Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs")
> Signed-off-by: Zhike Wang <wangzk320@163.com>
> ---
>  lib/librte_vhost/vhost_user.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 90ecee1..0cfb8b7 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -1563,8 +1563,10 @@
>  	struct virtio_net *dev = *pdev;
>  	struct vhost_vring_file file;
>  	struct vhost_virtqueue *vq;
> +	int expected_fds;
>  
> -	if (validate_msg_fds(msg, 1) != 0)
> +	expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> +	if (validate_msg_fds(msg, expected_fds) != 0)
>  		return RTE_VHOST_MSG_RESULT_ERR;
>  
>  	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> @@ -1588,7 +1590,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
>  			struct VhostUserMsg *msg,
>  			int main_fd __rte_unused)
>  {
> -	if (validate_msg_fds(msg, 1) != 0)
> +	int expected_fds;
> +
> +	expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> +	if (validate_msg_fds(msg, expected_fds) != 0)
>  		return RTE_VHOST_MSG_RESULT_ERR;
>  
>  	if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK))
> @@ -1790,8 +1795,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
>  	struct virtio_net *dev = *pdev;
>  	struct vhost_vring_file file;
>  	struct vhost_virtqueue *vq;
> +	int expected_fds;
>  
> -	if (validate_msg_fds(msg, 1) != 0)
> +	expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> +	if (validate_msg_fds(msg, expected_fds) != 0)
>  		return RTE_VHOST_MSG_RESULT_ERR;
>  
>  	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> 

Thanks for the fix, shame on me for missing that...

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Cc'ing stable also, as we'll need to backport it.

Regards,
Maxime
David Marchand Nov. 13, 2019, 10:26 a.m. UTC | #2 
On Wed, Nov 13, 2019 at 10:53 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Hi Zhike,
>
> On 11/13/19 6:03 AM, Zhike Wang wrote:
> > When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0.
> >
> > Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs")
> > Signed-off-by: Zhike Wang <wangzk320@163.com>
> > ---
> >  lib/librte_vhost/vhost_user.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > index 90ecee1..0cfb8b7 100644
> > --- a/lib/librte_vhost/vhost_user.c
> > +++ b/lib/librte_vhost/vhost_user.c
> > @@ -1563,8 +1563,10 @@
> >       struct virtio_net *dev = *pdev;
> >       struct vhost_vring_file file;
> >       struct vhost_virtqueue *vq;
> > +     int expected_fds;
> >
> > -     if (validate_msg_fds(msg, 1) != 0)
> > +     expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> > +     if (validate_msg_fds(msg, expected_fds) != 0)
> >               return RTE_VHOST_MSG_RESULT_ERR;
> >
> >       file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> > @@ -1588,7 +1590,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
> >                       struct VhostUserMsg *msg,
> >                       int main_fd __rte_unused)
> >  {
> > -     if (validate_msg_fds(msg, 1) != 0)
> > +     int expected_fds;
> > +
> > +     expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> > +     if (validate_msg_fds(msg, expected_fds) != 0)
> >               return RTE_VHOST_MSG_RESULT_ERR;
> >
> >       if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK))
> > @@ -1790,8 +1795,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
> >       struct virtio_net *dev = *pdev;
> >       struct vhost_vring_file file;
> >       struct vhost_virtqueue *vq;
> > +     int expected_fds;
> >
> > -     if (validate_msg_fds(msg, 1) != 0)
> > +     expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> > +     if (validate_msg_fds(msg, expected_fds) != 0)
> >               return RTE_VHOST_MSG_RESULT_ERR;
> >
> >       file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> >
>
> Thanks for the fix, shame on me for missing that...
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Cc'ing stable also, as we'll need to backport it.

Please, the title and the commitlog do not help to understand what the issue.
What is broken? Basic setups? Some specific setups and/or features?

Thanks.
王志克 Nov. 13, 2019, 11:04 a.m. UTC | #3 
Thanks for comment.
Just send out v2.

Br,
Zhike Wang 
JDCloud, Product Development, IaaS   
------------------------------------------------------------------------------------------------
Mobile／+86 13466719566
E- mail／wangzhike@jd.com
Address／5F Building A,North-Star Century Center,8 Beichen West Street,Chaoyang District Beijing
Https://JDCloud.com
------------------------------------------------------------------------------------------------



-----Original Message-----
From: David Marchand [mailto:david.marchand@redhat.com] 
Sent: Wednesday, November 13, 2019 6:26 PM
To: Maxime Coquelin; Zhike Wang
Cc: dev; security@dpdk.org; 王志克; stable@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] vhost: fix validate_msg_fds if VHOST_USER_VRING_NOFD_MASK set.

On Wed, Nov 13, 2019 at 10:53 AM Maxime Coquelin
<maxime.coquelin@redhat.com> wrote:
>
> Hi Zhike,
>
> On 11/13/19 6:03 AM, Zhike Wang wrote:
> > When VHOST_USER_VRING_NOFD_MASK is set, the fd_num is 0.
> >
> > Fixes: bf47225 ("vhost: fix possible denial of service by leaking FDs")
> > Signed-off-by: Zhike Wang <wangzk320@163.com>
> > ---
> >  lib/librte_vhost/vhost_user.c | 13 ++++++++++---
> >  1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> > index 90ecee1..0cfb8b7 100644
> > --- a/lib/librte_vhost/vhost_user.c
> > +++ b/lib/librte_vhost/vhost_user.c
> > @@ -1563,8 +1563,10 @@
> >       struct virtio_net *dev = *pdev;
> >       struct vhost_vring_file file;
> >       struct vhost_virtqueue *vq;
> > +     int expected_fds;
> >
> > -     if (validate_msg_fds(msg, 1) != 0)
> > +     expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> > +     if (validate_msg_fds(msg, expected_fds) != 0)
> >               return RTE_VHOST_MSG_RESULT_ERR;
> >
> >       file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> > @@ -1588,7 +1590,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
> >                       struct VhostUserMsg *msg,
> >                       int main_fd __rte_unused)
> >  {
> > -     if (validate_msg_fds(msg, 1) != 0)
> > +     int expected_fds;
> > +
> > +     expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> > +     if (validate_msg_fds(msg, expected_fds) != 0)
> >               return RTE_VHOST_MSG_RESULT_ERR;
> >
> >       if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK))
> > @@ -1790,8 +1795,10 @@ static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
> >       struct virtio_net *dev = *pdev;
> >       struct vhost_vring_file file;
> >       struct vhost_virtqueue *vq;
> > +     int expected_fds;
> >
> > -     if (validate_msg_fds(msg, 1) != 0)
> > +     expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
> > +     if (validate_msg_fds(msg, expected_fds) != 0)
> >               return RTE_VHOST_MSG_RESULT_ERR;
> >
> >       file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
> >
>
> Thanks for the fix, shame on me for missing that...
>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>
> Cc'ing stable also, as we'll need to backport it.

Please, the title and the commitlog do not help to understand what the issue.
What is broken? Basic setups? Some specific setups and/or features?

Thanks.
diff mbox series

Patch

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 90ecee1..0cfb8b7 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1563,8 +1563,10 @@ 
 	struct virtio_net *dev = *pdev;
 	struct vhost_vring_file file;
 	struct vhost_virtqueue *vq;
+	int expected_fds;
 
-	if (validate_msg_fds(msg, 1) != 0)
+	expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
+	if (validate_msg_fds(msg, expected_fds) != 0)
 		return RTE_VHOST_MSG_RESULT_ERR;
 
 	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;
@@ -1588,7 +1590,10 @@  static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
 			struct VhostUserMsg *msg,
 			int main_fd __rte_unused)
 {
-	if (validate_msg_fds(msg, 1) != 0)
+	int expected_fds;
+
+	expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
+	if (validate_msg_fds(msg, expected_fds) != 0)
 		return RTE_VHOST_MSG_RESULT_ERR;
 
 	if (!(msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK))
@@ -1790,8 +1795,10 @@  static int vhost_user_set_vring_err(struct virtio_net **pdev __rte_unused,
 	struct virtio_net *dev = *pdev;
 	struct vhost_vring_file file;
 	struct vhost_virtqueue *vq;
+	int expected_fds;
 
-	if (validate_msg_fds(msg, 1) != 0)
+	expected_fds = (msg->payload.u64 & VHOST_USER_VRING_NOFD_MASK) ? 0 : 1;
+	if (validate_msg_fds(msg, expected_fds) != 0)
 		return RTE_VHOST_MSG_RESULT_ERR;
 
 	file.index = msg->payload.u64 & VHOST_USER_VRING_IDX_MASK;