diff mbox series

virtiofsd: Fix potential NULL pointer dereference in virtio_send_msg()

Message ID 20201210151426.89244-1-alex.chen@huawei.com
State New
Headers show
Series virtiofsd: Fix potential NULL pointer dereference in virtio_send_msg() | expand

Commit Message

Alex Chen Dec. 10, 2020, 3:14 p.m. UTC
The 'ch' will be NULL in the following stack:
send_notify_iov()->fuse_send_msg()->virtio_send_msg(),
so we should check 'ch' is valid before dereferencing it

Reported-by: Euler Robot <euler.robot@huawei.com>
Signed-off-by: Alex Chen <alex.chen@huawei.com>
---
 tools/virtiofsd/fuse_virtio.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Dr. David Alan Gilbert Dec. 10, 2020, 6 p.m. UTC | #1
* Alex Chen (alex.chen@huawei.com) wrote:
> The 'ch' will be NULL in the following stack:
> send_notify_iov()->fuse_send_msg()->virtio_send_msg(),
> so we should check 'ch' is valid before dereferencing it
> 
> Reported-by: Euler Robot <euler.robot@huawei.com>
> Signed-off-by: Alex Chen <alex.chen@huawei.com>

Please check, but I don't think we can currently hit this because
we never call send_notify_iov - it's currently called by
fuse_lowlevel_notify_inval_entry, inval_inode, and
fuse_lowlevel_notify_poll -
but I don't think those are called anywhere.

In that case, probably the best fix is to remove those until we
put notify back in.

Dave

> ---
>  tools/virtiofsd/fuse_virtio.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
> index 623812c432..31b2187a15 100644
> --- a/tools/virtiofsd/fuse_virtio.c
> +++ b/tools/virtiofsd/fuse_virtio.c
> @@ -205,6 +205,8 @@ static void copy_iov(struct iovec *src_iov, int src_count,
>  int virtio_send_msg(struct fuse_session *se, struct fuse_chan *ch,
>                      struct iovec *iov, int count)
>  {
> +    assert(ch);
> +
>      FVRequest *req = container_of(ch, FVRequest, ch);
>      struct fv_QueueInfo *qi = ch->qi;
>      VuDev *dev = &se->virtio_dev->dev;
> -- 
> 2.19.1
>
Alex Chen Dec. 11, 2020, 10:16 a.m. UTC | #2
Hi Dave,

On 2020/12/11 2:00, Dr. David Alan Gilbert wrote:
> * Alex Chen (alex.chen@huawei.com) wrote:
>> The 'ch' will be NULL in the following stack:
>> send_notify_iov()->fuse_send_msg()->virtio_send_msg(),
>> so we should check 'ch' is valid before dereferencing it
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Alex Chen <alex.chen@huawei.com>
> 
> Please check, but I don't think we can currently hit this because
> we never call send_notify_iov - it's currently called by
> fuse_lowlevel_notify_inval_entry, inval_inode, and
> fuse_lowlevel_notify_poll -
> but I don't think those are called anywhere.
>

Thanks for your review.
The send_notify_iov() is really not being called.

> In that case, probably the best fix is to remove those until we
> put notify back in.
> 

OK, I will remove those temporarily useless code and send patch v2 to fix this problem.

Thanks,
Alex

> Dave
> 
>> ---
>>  tools/virtiofsd/fuse_virtio.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
>> index 623812c432..31b2187a15 100644
>> --- a/tools/virtiofsd/fuse_virtio.c
>> +++ b/tools/virtiofsd/fuse_virtio.c
>> @@ -205,6 +205,8 @@ static void copy_iov(struct iovec *src_iov, int src_count,
>>  int virtio_send_msg(struct fuse_session *se, struct fuse_chan *ch,
>>                      struct iovec *iov, int count)
>>  {
>> +    assert(ch);
>> +
>>      FVRequest *req = container_of(ch, FVRequest, ch);
>>      struct fv_QueueInfo *qi = ch->qi;
>>      VuDev *dev = &se->virtio_dev->dev;
>> -- 
>> 2.19.1
>>
diff mbox series

Patch

diff --git a/tools/virtiofsd/fuse_virtio.c b/tools/virtiofsd/fuse_virtio.c
index 623812c432..31b2187a15 100644
--- a/tools/virtiofsd/fuse_virtio.c
+++ b/tools/virtiofsd/fuse_virtio.c
@@ -205,6 +205,8 @@  static void copy_iov(struct iovec *src_iov, int src_count,
 int virtio_send_msg(struct fuse_session *se, struct fuse_chan *ch,
                     struct iovec *iov, int count)
 {
+    assert(ch);
+
     FVRequest *req = container_of(ch, FVRequest, ch);
     struct fv_QueueInfo *qi = ch->qi;
     VuDev *dev = &se->virtio_dev->dev;