* [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init()
@ 2020-10-22 14:51 Zhiqiang Liu
2020-10-27 9:21 ` Zhiqiang Liu
2020-10-29 15:25 ` Miklos Szeredi
0 siblings, 2 replies; 4+ messages in thread
From: Zhiqiang Liu @ 2020-10-22 14:51 UTC (permalink / raw)
To: miklos, mszeredi
Cc: linfeilong, linux-fsdevel, linux-kernel, lihaotian, liuzhiqiang26
In fuse_send_init func, ia is allocated by calling kzalloc func, and
we donot check whether ia is NULL before using it. Thus, if allocating
ia fails, accessing NULL pointer problem will occur.
Here, we will call process_init_reply func if ia is NULL.
Fixes: 615047eff108 ("fuse: convert init to simple api")
Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
Signed-off-by: Haotian Li <lihaotian9@huawei.com>
---
fs/fuse/inode.c | 161 ++++++++++++++++++++++++++----------------------
1 file changed, 87 insertions(+), 74 deletions(-)
diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 581329203d68..bb526d8cf5b0 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -898,88 +898,97 @@ struct fuse_init_args {
static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
int error)
{
- struct fuse_init_args *ia = container_of(args, typeof(*ia), args);
- struct fuse_init_out *arg = &ia->out;
+ struct fuse_init_args *ia;
+ struct fuse_init_out *arg;
+ unsigned long ra_pages;
- if (error || arg->major != FUSE_KERNEL_VERSION)
+ if (!args) {
fc->conn_error = 1;
- else {
- unsigned long ra_pages;
+ goto out;
+ }
- process_init_limits(fc, arg);
+ ia = container_of(args, typeof(*ia), args);
+ arg = &ia->out;
+ if (error || arg->major != FUSE_KERNEL_VERSION) {
+ fc->conn_error = 1;
+ goto out_free_ia;
+ }
- if (arg->minor >= 6) {
- ra_pages = arg->max_readahead / PAGE_SIZE;
- if (arg->flags & FUSE_ASYNC_READ)
- fc->async_read = 1;
- if (!(arg->flags & FUSE_POSIX_LOCKS))
- fc->no_lock = 1;
- if (arg->minor >= 17) {
- if (!(arg->flags & FUSE_FLOCK_LOCKS))
- fc->no_flock = 1;
- } else {
- if (!(arg->flags & FUSE_POSIX_LOCKS))
- fc->no_flock = 1;
- }
- if (arg->flags & FUSE_ATOMIC_O_TRUNC)
- fc->atomic_o_trunc = 1;
- if (arg->minor >= 9) {
- /* LOOKUP has dependency on proto version */
- if (arg->flags & FUSE_EXPORT_SUPPORT)
- fc->export_support = 1;
- }
- if (arg->flags & FUSE_BIG_WRITES)
- fc->big_writes = 1;
- if (arg->flags & FUSE_DONT_MASK)
- fc->dont_mask = 1;
- if (arg->flags & FUSE_AUTO_INVAL_DATA)
- fc->auto_inval_data = 1;
- else if (arg->flags & FUSE_EXPLICIT_INVAL_DATA)
- fc->explicit_inval_data = 1;
- if (arg->flags & FUSE_DO_READDIRPLUS) {
- fc->do_readdirplus = 1;
- if (arg->flags & FUSE_READDIRPLUS_AUTO)
- fc->readdirplus_auto = 1;
- }
- if (arg->flags & FUSE_ASYNC_DIO)
- fc->async_dio = 1;
- if (arg->flags & FUSE_WRITEBACK_CACHE)
- fc->writeback_cache = 1;
- if (arg->flags & FUSE_PARALLEL_DIROPS)
- fc->parallel_dirops = 1;
- if (arg->flags & FUSE_HANDLE_KILLPRIV)
- fc->handle_killpriv = 1;
- if (arg->time_gran && arg->time_gran <= 1000000000)
- fc->sb->s_time_gran = arg->time_gran;
- if ((arg->flags & FUSE_POSIX_ACL)) {
- fc->default_permissions = 1;
- fc->posix_acl = 1;
- fc->sb->s_xattr = fuse_acl_xattr_handlers;
- }
- if (arg->flags & FUSE_CACHE_SYMLINKS)
- fc->cache_symlinks = 1;
- if (arg->flags & FUSE_ABORT_ERROR)
- fc->abort_err = 1;
- if (arg->flags & FUSE_MAX_PAGES) {
- fc->max_pages =
- min_t(unsigned int, FUSE_MAX_MAX_PAGES,
- max_t(unsigned int, arg->max_pages, 1));
- }
- } else {
- ra_pages = fc->max_read / PAGE_SIZE;
+ process_init_limits(fc, arg);
+
+ if (arg->minor >= 6) {
+ ra_pages = arg->max_readahead / PAGE_SIZE;
+ if (arg->flags & FUSE_ASYNC_READ)
+ fc->async_read = 1;
+ if (!(arg->flags & FUSE_POSIX_LOCKS))
fc->no_lock = 1;
- fc->no_flock = 1;
+ if (arg->minor >= 17) {
+ if (!(arg->flags & FUSE_FLOCK_LOCKS))
+ fc->no_flock = 1;
+ } else {
+ if (!(arg->flags & FUSE_POSIX_LOCKS))
+ fc->no_flock = 1;
}
-
- fc->sb->s_bdi->ra_pages =
- min(fc->sb->s_bdi->ra_pages, ra_pages);
- fc->minor = arg->minor;
- fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
- fc->max_write = max_t(unsigned, 4096, fc->max_write);
- fc->conn_init = 1;
+ if (arg->flags & FUSE_ATOMIC_O_TRUNC)
+ fc->atomic_o_trunc = 1;
+ if (arg->minor >= 9) {
+ /* LOOKUP has dependency on proto version */
+ if (arg->flags & FUSE_EXPORT_SUPPORT)
+ fc->export_support = 1;
+ }
+ if (arg->flags & FUSE_BIG_WRITES)
+ fc->big_writes = 1;
+ if (arg->flags & FUSE_DONT_MASK)
+ fc->dont_mask = 1;
+ if (arg->flags & FUSE_AUTO_INVAL_DATA)
+ fc->auto_inval_data = 1;
+ else if (arg->flags & FUSE_EXPLICIT_INVAL_DATA)
+ fc->explicit_inval_data = 1;
+ if (arg->flags & FUSE_DO_READDIRPLUS) {
+ fc->do_readdirplus = 1;
+ if (arg->flags & FUSE_READDIRPLUS_AUTO)
+ fc->readdirplus_auto = 1;
+ }
+ if (arg->flags & FUSE_ASYNC_DIO)
+ fc->async_dio = 1;
+ if (arg->flags & FUSE_WRITEBACK_CACHE)
+ fc->writeback_cache = 1;
+ if (arg->flags & FUSE_PARALLEL_DIROPS)
+ fc->parallel_dirops = 1;
+ if (arg->flags & FUSE_HANDLE_KILLPRIV)
+ fc->handle_killpriv = 1;
+ if (arg->time_gran && arg->time_gran <= 1000000000)
+ fc->sb->s_time_gran = arg->time_gran;
+ if ((arg->flags & FUSE_POSIX_ACL)) {
+ fc->default_permissions = 1;
+ fc->posix_acl = 1;
+ fc->sb->s_xattr = fuse_acl_xattr_handlers;
+ }
+ if (arg->flags & FUSE_CACHE_SYMLINKS)
+ fc->cache_symlinks = 1;
+ if (arg->flags & FUSE_ABORT_ERROR)
+ fc->abort_err = 1;
+ if (arg->flags & FUSE_MAX_PAGES) {
+ fc->max_pages =
+ min_t(unsigned int, FUSE_MAX_MAX_PAGES,
+ max_t(unsigned int, arg->max_pages, 1));
+ }
+ } else {
+ ra_pages = fc->max_read / PAGE_SIZE;
+ fc->no_lock = 1;
+ fc->no_flock = 1;
}
- kfree(ia);
+ fc->sb->s_bdi->ra_pages =
+ min(fc->sb->s_bdi->ra_pages, ra_pages);
+ fc->minor = arg->minor;
+ fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
+ fc->max_write = max_t(unsigned int, 4096, fc->max_write);
+ fc->conn_init = 1;
+
+out_free_ia:
+ kfree(ia);
+out:
fuse_set_initialized(fc);
wake_up_all(&fc->blocked_waitq);
}
@@ -989,6 +998,10 @@ void fuse_send_init(struct fuse_conn *fc)
struct fuse_init_args *ia;
ia = kzalloc(sizeof(*ia), GFP_KERNEL | __GFP_NOFAIL);
+ if (!ia) {
+ process_init_reply(fc, NULL, -ENOTCONN);
+ return;
+ }
ia->in.major = FUSE_KERNEL_VERSION;
ia->in.minor = FUSE_KERNEL_MINOR_VERSION;
--
2.19.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init()
2020-10-22 14:51 [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init() Zhiqiang Liu
@ 2020-10-27 9:21 ` Zhiqiang Liu
2020-10-29 15:25 ` Miklos Szeredi
1 sibling, 0 replies; 4+ messages in thread
From: Zhiqiang Liu @ 2020-10-27 9:21 UTC (permalink / raw)
To: miklos, mszeredi
Cc: linfeilong, linux-fsdevel, linux-kernel, lihaotian, fuse-devel,
kschalk, Nikolaus
friendly ping...
On 2020/10/22 22:51, Zhiqiang Liu wrote:
>
> In fuse_send_init func, ia is allocated by calling kzalloc func, and
> we donot check whether ia is NULL before using it. Thus, if allocating
> ia fails, accessing NULL pointer problem will occur.
>
> Here, we will call process_init_reply func if ia is NULL.
>
> Fixes: 615047eff108 ("fuse: convert init to simple api")
> Signed-off-by: Zhiqiang Liu <liuzhiqiang26@huawei.com>
> Signed-off-by: Haotian Li <lihaotian9@huawei.com>
> ---
> fs/fuse/inode.c | 161 ++++++++++++++++++++++++++----------------------
> 1 file changed, 87 insertions(+), 74 deletions(-)
>
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index 581329203d68..bb526d8cf5b0 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
> @@ -898,88 +898,97 @@ struct fuse_init_args {
> static void process_init_reply(struct fuse_conn *fc, struct fuse_args *args,
> int error)
> {
> - struct fuse_init_args *ia = container_of(args, typeof(*ia), args);
> - struct fuse_init_out *arg = &ia->out;
> + struct fuse_init_args *ia;
> + struct fuse_init_out *arg;
> + unsigned long ra_pages;
>
> - if (error || arg->major != FUSE_KERNEL_VERSION)
> + if (!args) {
> fc->conn_error = 1;
> - else {
> - unsigned long ra_pages;
> + goto out;
> + }
>
> - process_init_limits(fc, arg);
> + ia = container_of(args, typeof(*ia), args);
> + arg = &ia->out;
> + if (error || arg->major != FUSE_KERNEL_VERSION) {
> + fc->conn_error = 1;
> + goto out_free_ia;
> + }
>
> - if (arg->minor >= 6) {
> - ra_pages = arg->max_readahead / PAGE_SIZE;
> - if (arg->flags & FUSE_ASYNC_READ)
> - fc->async_read = 1;
> - if (!(arg->flags & FUSE_POSIX_LOCKS))
> - fc->no_lock = 1;
> - if (arg->minor >= 17) {
> - if (!(arg->flags & FUSE_FLOCK_LOCKS))
> - fc->no_flock = 1;
> - } else {
> - if (!(arg->flags & FUSE_POSIX_LOCKS))
> - fc->no_flock = 1;
> - }
> - if (arg->flags & FUSE_ATOMIC_O_TRUNC)
> - fc->atomic_o_trunc = 1;
> - if (arg->minor >= 9) {
> - /* LOOKUP has dependency on proto version */
> - if (arg->flags & FUSE_EXPORT_SUPPORT)
> - fc->export_support = 1;
> - }
> - if (arg->flags & FUSE_BIG_WRITES)
> - fc->big_writes = 1;
> - if (arg->flags & FUSE_DONT_MASK)
> - fc->dont_mask = 1;
> - if (arg->flags & FUSE_AUTO_INVAL_DATA)
> - fc->auto_inval_data = 1;
> - else if (arg->flags & FUSE_EXPLICIT_INVAL_DATA)
> - fc->explicit_inval_data = 1;
> - if (arg->flags & FUSE_DO_READDIRPLUS) {
> - fc->do_readdirplus = 1;
> - if (arg->flags & FUSE_READDIRPLUS_AUTO)
> - fc->readdirplus_auto = 1;
> - }
> - if (arg->flags & FUSE_ASYNC_DIO)
> - fc->async_dio = 1;
> - if (arg->flags & FUSE_WRITEBACK_CACHE)
> - fc->writeback_cache = 1;
> - if (arg->flags & FUSE_PARALLEL_DIROPS)
> - fc->parallel_dirops = 1;
> - if (arg->flags & FUSE_HANDLE_KILLPRIV)
> - fc->handle_killpriv = 1;
> - if (arg->time_gran && arg->time_gran <= 1000000000)
> - fc->sb->s_time_gran = arg->time_gran;
> - if ((arg->flags & FUSE_POSIX_ACL)) {
> - fc->default_permissions = 1;
> - fc->posix_acl = 1;
> - fc->sb->s_xattr = fuse_acl_xattr_handlers;
> - }
> - if (arg->flags & FUSE_CACHE_SYMLINKS)
> - fc->cache_symlinks = 1;
> - if (arg->flags & FUSE_ABORT_ERROR)
> - fc->abort_err = 1;
> - if (arg->flags & FUSE_MAX_PAGES) {
> - fc->max_pages =
> - min_t(unsigned int, FUSE_MAX_MAX_PAGES,
> - max_t(unsigned int, arg->max_pages, 1));
> - }
> - } else {
> - ra_pages = fc->max_read / PAGE_SIZE;
> + process_init_limits(fc, arg);
> +
> + if (arg->minor >= 6) {
> + ra_pages = arg->max_readahead / PAGE_SIZE;
> + if (arg->flags & FUSE_ASYNC_READ)
> + fc->async_read = 1;
> + if (!(arg->flags & FUSE_POSIX_LOCKS))
> fc->no_lock = 1;
> - fc->no_flock = 1;
> + if (arg->minor >= 17) {
> + if (!(arg->flags & FUSE_FLOCK_LOCKS))
> + fc->no_flock = 1;
> + } else {
> + if (!(arg->flags & FUSE_POSIX_LOCKS))
> + fc->no_flock = 1;
> }
> -
> - fc->sb->s_bdi->ra_pages =
> - min(fc->sb->s_bdi->ra_pages, ra_pages);
> - fc->minor = arg->minor;
> - fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
> - fc->max_write = max_t(unsigned, 4096, fc->max_write);
> - fc->conn_init = 1;
> + if (arg->flags & FUSE_ATOMIC_O_TRUNC)
> + fc->atomic_o_trunc = 1;
> + if (arg->minor >= 9) {
> + /* LOOKUP has dependency on proto version */
> + if (arg->flags & FUSE_EXPORT_SUPPORT)
> + fc->export_support = 1;
> + }
> + if (arg->flags & FUSE_BIG_WRITES)
> + fc->big_writes = 1;
> + if (arg->flags & FUSE_DONT_MASK)
> + fc->dont_mask = 1;
> + if (arg->flags & FUSE_AUTO_INVAL_DATA)
> + fc->auto_inval_data = 1;
> + else if (arg->flags & FUSE_EXPLICIT_INVAL_DATA)
> + fc->explicit_inval_data = 1;
> + if (arg->flags & FUSE_DO_READDIRPLUS) {
> + fc->do_readdirplus = 1;
> + if (arg->flags & FUSE_READDIRPLUS_AUTO)
> + fc->readdirplus_auto = 1;
> + }
> + if (arg->flags & FUSE_ASYNC_DIO)
> + fc->async_dio = 1;
> + if (arg->flags & FUSE_WRITEBACK_CACHE)
> + fc->writeback_cache = 1;
> + if (arg->flags & FUSE_PARALLEL_DIROPS)
> + fc->parallel_dirops = 1;
> + if (arg->flags & FUSE_HANDLE_KILLPRIV)
> + fc->handle_killpriv = 1;
> + if (arg->time_gran && arg->time_gran <= 1000000000)
> + fc->sb->s_time_gran = arg->time_gran;
> + if ((arg->flags & FUSE_POSIX_ACL)) {
> + fc->default_permissions = 1;
> + fc->posix_acl = 1;
> + fc->sb->s_xattr = fuse_acl_xattr_handlers;
> + }
> + if (arg->flags & FUSE_CACHE_SYMLINKS)
> + fc->cache_symlinks = 1;
> + if (arg->flags & FUSE_ABORT_ERROR)
> + fc->abort_err = 1;
> + if (arg->flags & FUSE_MAX_PAGES) {
> + fc->max_pages =
> + min_t(unsigned int, FUSE_MAX_MAX_PAGES,
> + max_t(unsigned int, arg->max_pages, 1));
> + }
> + } else {
> + ra_pages = fc->max_read / PAGE_SIZE;
> + fc->no_lock = 1;
> + fc->no_flock = 1;
> }
> - kfree(ia);
>
> + fc->sb->s_bdi->ra_pages =
> + min(fc->sb->s_bdi->ra_pages, ra_pages);
> + fc->minor = arg->minor;
> + fc->max_write = arg->minor < 5 ? 4096 : arg->max_write;
> + fc->max_write = max_t(unsigned int, 4096, fc->max_write);
> + fc->conn_init = 1;
> +
> +out_free_ia:
> + kfree(ia);
> +out:
> fuse_set_initialized(fc);
> wake_up_all(&fc->blocked_waitq);
> }
> @@ -989,6 +998,10 @@ void fuse_send_init(struct fuse_conn *fc)
> struct fuse_init_args *ia;
>
> ia = kzalloc(sizeof(*ia), GFP_KERNEL | __GFP_NOFAIL);
> + if (!ia) {
> + process_init_reply(fc, NULL, -ENOTCONN);
> + return;
> + }
>
> ia->in.major = FUSE_KERNEL_VERSION;
> ia->in.minor = FUSE_KERNEL_MINOR_VERSION;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init()
2020-10-22 14:51 [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init() Zhiqiang Liu
2020-10-27 9:21 ` Zhiqiang Liu
@ 2020-10-29 15:25 ` Miklos Szeredi
2020-10-30 3:01 ` Zhiqiang Liu
1 sibling, 1 reply; 4+ messages in thread
From: Miklos Szeredi @ 2020-10-29 15:25 UTC (permalink / raw)
To: Zhiqiang Liu
Cc: Miklos Szeredi, linfeilong, linux-fsdevel, linux-kernel, lihaotian
On Thu, Oct 22, 2020 at 4:52 PM Zhiqiang Liu <liuzhiqiang26@huawei.com> wrote:
>
>
> In fuse_send_init func, ia is allocated by calling kzalloc func, and
> we donot check whether ia is NULL before using it. Thus, if allocating
> ia fails, accessing NULL pointer problem will occur.
Note the __GFP_NOFAIL flag for kzalloc(), which ensures that it will not fail.
Thanks,
Miklos
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init()
2020-10-29 15:25 ` Miklos Szeredi
@ 2020-10-30 3:01 ` Zhiqiang Liu
0 siblings, 0 replies; 4+ messages in thread
From: Zhiqiang Liu @ 2020-10-30 3:01 UTC (permalink / raw)
To: Miklos Szeredi
Cc: Miklos Szeredi, linfeilong, linux-fsdevel, linux-kernel, lihaotian
On 2020/10/29 23:25, Miklos Szeredi wrote:
> On Thu, Oct 22, 2020 at 4:52 PM Zhiqiang Liu <liuzhiqiang26@huawei.com> wrote:
>>
>>
>> In fuse_send_init func, ia is allocated by calling kzalloc func, and
>> we donot check whether ia is NULL before using it. Thus, if allocating
>> ia fails, accessing NULL pointer problem will occur.
>
> Note the __GFP_NOFAIL flag for kzalloc(), which ensures that it will not fail.
Thanks for your reply.
Please ignore this patch.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-10-30 3:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-22 14:51 [PATCH] fuse: fix potential accessing NULL pointer problem in fuse_send_init() Zhiqiang Liu
2020-10-27 9:21 ` Zhiqiang Liu
2020-10-29 15:25 ` Miklos Szeredi
2020-10-30 3:01 ` Zhiqiang Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).