* [PATCH v3 0/3] virtiofsd: fix some accessing NULL pointer problem
@ 2020-11-11 1:04 ` Haotian Li
0 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:04 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong, liuzhiqiang26
Hi,
We find some potential NULL pointer bugs on tools/virtiofsd.
Three patches are made to fix them
Haotian Li (3):
tools/virtiofsd/buffer.c: check whether buf is NULL in
fuse_bufvec_advance func
virtiofsd: check whether lo_map_reserve returns NULL in main func
virtiofsd: check whether strdup lo.source return NULL in main func.
tools/virtiofsd/buffer.c | 4 ++++
tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
--
^ permalink raw reply [flat|nested] 13+ messages in thread
* [Virtio-fs] [PATCH v3 0/3] virtiofsd: fix some accessing NULL pointer problem
@ 2020-11-11 1:04 ` Haotian Li
0 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:04 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong
Hi,
We find some potential NULL pointer bugs on tools/virtiofsd.
Three patches are made to fix them
Haotian Li (3):
tools/virtiofsd/buffer.c: check whether buf is NULL in
fuse_bufvec_advance func
virtiofsd: check whether lo_map_reserve returns NULL in main func
virtiofsd: check whether strdup lo.source return NULL in main func.
tools/virtiofsd/buffer.c | 4 ++++
tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
--
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 1/3] tools/virtiofsd/buffer.c: check whether buf is NULL in fuse_bufvec_advance func
2020-11-11 1:04 ` [Virtio-fs] " Haotian Li
@ 2020-11-11 1:05 ` Haotian Li
-1 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:05 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong, liuzhiqiang26
In fuse_bufvec_advance func, calling fuse_bufvec_current func
may return NULL, so we should check whether buf is NULL before
using it.
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
tools/virtiofsd/buffer.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virtiofsd/buffer.c b/tools/virtiofsd/buffer.c
index 27c1377f22..bdc608c221 100644
--- a/tools/virtiofsd/buffer.c
+++ b/tools/virtiofsd/buffer.c
@@ -246,6 +246,10 @@ static int fuse_bufvec_advance(struct fuse_bufvec *bufv, size_t len)
{
const struct fuse_buf *buf = fuse_bufvec_current(bufv);
+ if (!buf) {
+ return 0;
+ }
+
bufv->off += len;
assert(bufv->off <= buf->size);
if (bufv->off == buf->size) {
--
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Virtio-fs] [PATCH 1/3] tools/virtiofsd/buffer.c: check whether buf is NULL in fuse_bufvec_advance func
@ 2020-11-11 1:05 ` Haotian Li
0 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:05 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong
In fuse_bufvec_advance func, calling fuse_bufvec_current func
may return NULL, so we should check whether buf is NULL before
using it.
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
tools/virtiofsd/buffer.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virtiofsd/buffer.c b/tools/virtiofsd/buffer.c
index 27c1377f22..bdc608c221 100644
--- a/tools/virtiofsd/buffer.c
+++ b/tools/virtiofsd/buffer.c
@@ -246,6 +246,10 @@ static int fuse_bufvec_advance(struct fuse_bufvec *bufv, size_t len)
{
const struct fuse_buf *buf = fuse_bufvec_current(bufv);
+ if (!buf) {
+ return 0;
+ }
+
bufv->off += len;
assert(bufv->off <= buf->size);
if (bufv->off == buf->size) {
--
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 2/3] virtiofsd: check whether lo_map_reserve returns NULL in, main func
2020-11-11 1:04 ` [Virtio-fs] " Haotian Li
@ 2020-11-11 1:09 ` Haotian Li
-1 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:09 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong, liuzhiqiang26
In main func, func lo_map_reserve is called without NULL check.
If reallocing new_elems fails in func lo_map_grow, the func
lo_map_reserve may return NULL. We should check whether
lo_map_reserve returns NULL before using it.
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
tools/virtiofsd/passthrough_ll.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index ec1008bceb..3e9bbc7a04 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3433,6 +3433,7 @@ int main(int argc, char *argv[])
.proc_self_fd = -1,
};
struct lo_map_elem *root_elem;
+ struct lo_map_elem *reserve_elem;
int ret = -1;
/* Don't mask creation mode, kernel already did that */
@@ -3452,8 +3453,17 @@ int main(int argc, char *argv[])
* [1] Root inode
*/
lo_map_init(&lo.ino_map);
- lo_map_reserve(&lo.ino_map, 0)->in_use = false;
+ reserve_elem = lo_map_reserve(&lo.ino_map, 0);
+ if (!reserve_elem) {
+ fuse_log(FUSE_LOG_ERR, "failed to alloc reserve_elem.\n");
+ goto err_out1;
+ }
+ reserve_elem->in_use = false;
root_elem = lo_map_reserve(&lo.ino_map, lo.root.fuse_ino);
+ if (!root_elem) {
+ fuse_log(FUSE_LOG_ERR, "failed to alloc root_elem.\n");
+ goto err_out1;
+ }
root_elem->inode = &lo.root;
lo_map_init(&lo.dirp_map);
--
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Virtio-fs] [PATCH 2/3] virtiofsd: check whether lo_map_reserve returns NULL in, main func
@ 2020-11-11 1:09 ` Haotian Li
0 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:09 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong
In main func, func lo_map_reserve is called without NULL check.
If reallocing new_elems fails in func lo_map_grow, the func
lo_map_reserve may return NULL. We should check whether
lo_map_reserve returns NULL before using it.
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
tools/virtiofsd/passthrough_ll.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index ec1008bceb..3e9bbc7a04 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3433,6 +3433,7 @@ int main(int argc, char *argv[])
.proc_self_fd = -1,
};
struct lo_map_elem *root_elem;
+ struct lo_map_elem *reserve_elem;
int ret = -1;
/* Don't mask creation mode, kernel already did that */
@@ -3452,8 +3453,17 @@ int main(int argc, char *argv[])
* [1] Root inode
*/
lo_map_init(&lo.ino_map);
- lo_map_reserve(&lo.ino_map, 0)->in_use = false;
+ reserve_elem = lo_map_reserve(&lo.ino_map, 0);
+ if (!reserve_elem) {
+ fuse_log(FUSE_LOG_ERR, "failed to alloc reserve_elem.\n");
+ goto err_out1;
+ }
+ reserve_elem->in_use = false;
root_elem = lo_map_reserve(&lo.ino_map, lo.root.fuse_ino);
+ if (!root_elem) {
+ fuse_log(FUSE_LOG_ERR, "failed to alloc root_elem.\n");
+ goto err_out1;
+ }
root_elem->inode = &lo.root;
lo_map_init(&lo.dirp_map);
--
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/3] virtiofsd: check whether strdup lo.source return NULL in main func
2020-11-11 1:04 ` [Virtio-fs] " Haotian Li
@ 2020-11-11 1:10 ` Haotian Li
-1 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:10 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong, liuzhiqiang26
In main func, strdup lo.source may fail. So check whether strdup
lo.source return NULL before using it.
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
tools/virtiofsd/passthrough_ll.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 3e9bbc7a04..0c11134fb5 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3525,6 +3525,10 @@ int main(int argc, char *argv[])
}
} else {
lo.source = strdup("/");
+ if (!lo.source) {
+ fuse_log(FUSE_LOG_ERR, "failed to strdup source\n");
+ goto err_out1;
+ }
}
if (lo.xattrmap) {
--
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Virtio-fs] [PATCH 3/3] virtiofsd: check whether strdup lo.source return NULL in main func
@ 2020-11-11 1:10 ` Haotian Li
0 siblings, 0 replies; 13+ messages in thread
From: Haotian Li @ 2020-11-11 1:10 UTC (permalink / raw)
To: qemu-devel, virtio-fs; +Cc: linfeilong
In main func, strdup lo.source may fail. So check whether strdup
lo.source return NULL before using it.
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
---
tools/virtiofsd/passthrough_ll.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
index 3e9bbc7a04..0c11134fb5 100644
--- a/tools/virtiofsd/passthrough_ll.c
+++ b/tools/virtiofsd/passthrough_ll.c
@@ -3525,6 +3525,10 @@ int main(int argc, char *argv[])
}
} else {
lo.source = strdup("/");
+ if (!lo.source) {
+ fuse_log(FUSE_LOG_ERR, "failed to strdup source\n");
+ goto err_out1;
+ }
}
if (lo.xattrmap) {
--
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Virtio-fs] [PATCH 2/3] virtiofsd: check whether lo_map_reserve returns NULL in, main func
2020-11-11 1:09 ` [Virtio-fs] " Haotian Li
(?)
@ 2020-11-11 19:52 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 13+ messages in thread
From: Dr. David Alan Gilbert @ 2020-11-11 19:52 UTC (permalink / raw)
To: Haotian Li; +Cc: virtio-fs, linfeilong, qemu-devel
* Haotian Li (lihaotian9@huawei.com) wrote:
> In main func, func lo_map_reserve is called without NULL check.
> If reallocing new_elems fails in func lo_map_grow, the func
> lo_map_reserve may return NULL. We should check whether
> lo_map_reserve returns NULL before using it.
>
> Signed-off-by: Haotian Li <lihaotian9@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
(I think the fuse_log will always fall through the default_log_func
becuase it's very early)
> ---
> tools/virtiofsd/passthrough_ll.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index ec1008bceb..3e9bbc7a04 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -3433,6 +3433,7 @@ int main(int argc, char *argv[])
> .proc_self_fd = -1,
> };
> struct lo_map_elem *root_elem;
> + struct lo_map_elem *reserve_elem;
> int ret = -1;
>
> /* Don't mask creation mode, kernel already did that */
> @@ -3452,8 +3453,17 @@ int main(int argc, char *argv[])
> * [1] Root inode
> */
> lo_map_init(&lo.ino_map);
> - lo_map_reserve(&lo.ino_map, 0)->in_use = false;
> + reserve_elem = lo_map_reserve(&lo.ino_map, 0);
> + if (!reserve_elem) {
> + fuse_log(FUSE_LOG_ERR, "failed to alloc reserve_elem.\n");
> + goto err_out1;
> + }
> + reserve_elem->in_use = false;
> root_elem = lo_map_reserve(&lo.ino_map, lo.root.fuse_ino);
> + if (!root_elem) {
> + fuse_log(FUSE_LOG_ERR, "failed to alloc root_elem.\n");
> + goto err_out1;
> + }
> root_elem->inode = &lo.root;
>
> lo_map_init(&lo.dirp_map);
> --
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Virtio-fs] [PATCH 3/3] virtiofsd: check whether strdup lo.source return NULL in main func
2020-11-11 1:10 ` [Virtio-fs] " Haotian Li
(?)
@ 2020-11-11 19:55 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 13+ messages in thread
From: Dr. David Alan Gilbert @ 2020-11-11 19:55 UTC (permalink / raw)
To: Haotian Li; +Cc: virtio-fs, linfeilong, qemu-devel
* Haotian Li (lihaotian9@huawei.com) wrote:
> In main func, strdup lo.source may fail. So check whether strdup
> lo.source return NULL before using it.
>
> Signed-off-by: Haotian Li <lihaotian9@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
> tools/virtiofsd/passthrough_ll.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/virtiofsd/passthrough_ll.c b/tools/virtiofsd/passthrough_ll.c
> index 3e9bbc7a04..0c11134fb5 100644
> --- a/tools/virtiofsd/passthrough_ll.c
> +++ b/tools/virtiofsd/passthrough_ll.c
> @@ -3525,6 +3525,10 @@ int main(int argc, char *argv[])
> }
> } else {
> lo.source = strdup("/");
> + if (!lo.source) {
> + fuse_log(FUSE_LOG_ERR, "failed to strdup source\n");
> + goto err_out1;
> + }
> }
>
(It's interesting we use exit's in some places, goto's in others)
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> if (lo.xattrmap) {
> --
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Virtio-fs] [PATCH 1/3] tools/virtiofsd/buffer.c: check whether buf is NULL in fuse_bufvec_advance func
2020-11-11 1:05 ` [Virtio-fs] " Haotian Li
(?)
@ 2020-11-11 19:59 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 13+ messages in thread
From: Dr. David Alan Gilbert @ 2020-11-11 19:59 UTC (permalink / raw)
To: Haotian Li; +Cc: virtio-fs, linfeilong, qemu-devel
* Haotian Li (lihaotian9@huawei.com) wrote:
> In fuse_bufvec_advance func, calling fuse_bufvec_current func
> may return NULL, so we should check whether buf is NULL before
> using it.
>
> Signed-off-by: Haotian Li <lihaotian9@huawei.com>
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> ---
> tools/virtiofsd/buffer.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/virtiofsd/buffer.c b/tools/virtiofsd/buffer.c
> index 27c1377f22..bdc608c221 100644
> --- a/tools/virtiofsd/buffer.c
> +++ b/tools/virtiofsd/buffer.c
> @@ -246,6 +246,10 @@ static int fuse_bufvec_advance(struct fuse_bufvec *bufv, size_t len)
> {
> const struct fuse_buf *buf = fuse_bufvec_current(bufv);
>
> + if (!buf) {
> + return 0;
> + }
> +
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> bufv->off += len;
> assert(bufv->off <= buf->size);
> if (bufv->off == buf->size) {
> --
>
> _______________________________________________
> Virtio-fs mailing list
> Virtio-fs@redhat.com
> https://www.redhat.com/mailman/listinfo/virtio-fs
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 0/3] virtiofsd: fix some accessing NULL pointer problem
2020-11-11 1:04 ` [Virtio-fs] " Haotian Li
@ 2020-11-12 16:25 ` Dr. David Alan Gilbert
-1 siblings, 0 replies; 13+ messages in thread
From: Dr. David Alan Gilbert @ 2020-11-12 16:25 UTC (permalink / raw)
To: Haotian Li; +Cc: virtio-fs, linfeilong, qemu-devel, liuzhiqiang26
* Haotian Li (lihaotian9@huawei.com) wrote:
> Hi,
> We find some potential NULL pointer bugs on tools/virtiofsd.
> Three patches are made to fix them
Queued
> Haotian Li (3):
> tools/virtiofsd/buffer.c: check whether buf is NULL in
> fuse_bufvec_advance func
> virtiofsd: check whether lo_map_reserve returns NULL in main func
> virtiofsd: check whether strdup lo.source return NULL in main func.
>
> tools/virtiofsd/buffer.c | 4 ++++
> tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> --
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Virtio-fs] [PATCH v3 0/3] virtiofsd: fix some accessing NULL pointer problem
@ 2020-11-12 16:25 ` Dr. David Alan Gilbert
0 siblings, 0 replies; 13+ messages in thread
From: Dr. David Alan Gilbert @ 2020-11-12 16:25 UTC (permalink / raw)
To: Haotian Li; +Cc: virtio-fs, linfeilong, qemu-devel
* Haotian Li (lihaotian9@huawei.com) wrote:
> Hi,
> We find some potential NULL pointer bugs on tools/virtiofsd.
> Three patches are made to fix them
Queued
> Haotian Li (3):
> tools/virtiofsd/buffer.c: check whether buf is NULL in
> fuse_bufvec_advance func
> virtiofsd: check whether lo_map_reserve returns NULL in main func
> virtiofsd: check whether strdup lo.source return NULL in main func.
>
> tools/virtiofsd/buffer.c | 4 ++++
> tools/virtiofsd/passthrough_ll.c | 16 +++++++++++++++-
> 2 files changed, 19 insertions(+), 1 deletion(-)
>
> --
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2020-11-12 16:26 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 1:04 [PATCH v3 0/3] virtiofsd: fix some accessing NULL pointer problem Haotian Li
2020-11-11 1:04 ` [Virtio-fs] " Haotian Li
2020-11-11 1:05 ` [PATCH 1/3] tools/virtiofsd/buffer.c: check whether buf is NULL in fuse_bufvec_advance func Haotian Li
2020-11-11 1:05 ` [Virtio-fs] " Haotian Li
2020-11-11 19:59 ` Dr. David Alan Gilbert
2020-11-11 1:09 ` [PATCH 2/3] virtiofsd: check whether lo_map_reserve returns NULL in, main func Haotian Li
2020-11-11 1:09 ` [Virtio-fs] " Haotian Li
2020-11-11 19:52 ` Dr. David Alan Gilbert
2020-11-11 1:10 ` [PATCH 3/3] virtiofsd: check whether strdup lo.source return NULL in " Haotian Li
2020-11-11 1:10 ` [Virtio-fs] " Haotian Li
2020-11-11 19:55 ` Dr. David Alan Gilbert
2020-11-12 16:25 ` [PATCH v3 0/3] virtiofsd: fix some accessing NULL pointer problem Dr. David Alan Gilbert
2020-11-12 16:25 ` [Virtio-fs] " Dr. David Alan Gilbert
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.