Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fuse: Resolve asan bug in during receive event notification #4019

Merged
merged 2 commits into from Mar 1, 2023

Conversation

mohit84
Copy link
Contributor

@mohit84 mohit84 commented Mar 1, 2023

The fuse xlator notify function tries to assign data object to graph object without checking an event.
In case of upcall event data object represents upcall object so during access of graph object the
process crashed for asan build.

Solution: Access the graph->id only while an event is associated specifically to fuse xlator

Fixes: #3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf

@mohit84 mohit84 requested a review from xhernandez March 1, 2023 05:28
@mohit84
Copy link
Contributor Author

mohit84 commented Mar 1, 2023

/run regression

@gluster-ant
Copy link
Collaborator

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index a9ffbc06e..c88e0087e 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -6352,7 +6352,8 @@ fuse_priv_dump(xlator_t *this)
     if (!this)
         return -1;
 
-    private = this->private;
+   private
+    = this->private;
 
     if (!private)
         return -1;
@@ -6506,7 +6507,8 @@ notify(xlator_t *this, int32_t event, void *data, ...)
     glusterfs_graph_t *graph = NULL;
     struct pollfd pfd = {0};
 
-    private = this->private;
+   private
+    = this->private;
 
     graph = data;
 
@@ -6528,7 +6530,8 @@ notify(xlator_t *this, int32_t event, void *data, ...)
                 (event == GF_EVENT_CHILD_DOWN)) {
                 pthread_mutex_lock(&private->sync_mutex);
                 {
-                    private->event_recvd = 1;
+                   private
+                    ->event_recvd = 1;
                     pthread_cond_broadcast(&private->sync_cond);
                 }
                 pthread_mutex_unlock(&private->sync_mutex);
@@ -6537,16 +6540,18 @@ notify(xlator_t *this, int32_t event, void *data, ...)
             pthread_mutex_lock(&private->sync_mutex);
             {
                 if (!private->fuse_thread_started) {
-                    private->fuse_thread_started = 1;
+                   private
+                    ->fuse_thread_started = 1;
                     start_thread = _gf_true;
                 }
             }
             pthread_mutex_unlock(&private->sync_mutex);
 
             if (start_thread) {
-                private->fuse_thread = GF_CALLOC(private->reader_thread_count,
-                                                 sizeof(pthread_t),
-                                                 gf_fuse_mt_pthread_t);
+               private
+                ->fuse_thread = GF_CALLOC(private->reader_thread_count,
+                                          sizeof(pthread_t),
+                                          gf_fuse_mt_pthread_t);
                 for (i = 0; i < private->reader_thread_count; i++) {
                     ret = gf_thread_create(&private->fuse_thread[i], NULL,
                                            fuse_thread_proc, this, "fuseproc");
@@ -6580,7 +6585,8 @@ notify(xlator_t *this, int32_t event, void *data, ...)
                         if (fuse_get_mount_status(this) != 0) {
                             goto auth_fail_unlock;
                         }
-                        private->mount_finished = _gf_true;
+                       private
+                        ->mount_finished = _gf_true;
                     } else if (pfd.revents) {
                         gf_log(this->name, GF_LOG_ERROR,
                                "mount pipe closed without status");

break;
}

gf_log("fuse", GF_LOG_DEBUG, "got event %d on graph %d", event,
((graph && event_graph) ? graph->id : 0));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since '0' is a valid graph id, when the graph is not known I would use a different value (maybe '-1') or write different log messages depending on event_graph value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

The fuse xlator notify function tries to assign data object
to graph object without checking an event. In case of upcall
event data object represents upcall object so during access
of graph object the process is crashed for asan build.

Solution: Access the graph->id only while event is associated
          specific to fuse xlator

Fixes: gluster#3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
Fixes: gluster#3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
@mohit84
Copy link
Contributor Author

mohit84 commented Mar 1, 2023

/run regression

@gluster-ant
Copy link
Collaborator

CLANG-FORMAT FAILURE:
Before merging the patch, this diff needs to be considered for passing clang-format

index 2dc9b4f42..c96b85c45 100644
--- a/xlators/mount/fuse/src/fuse-bridge.c
+++ b/xlators/mount/fuse/src/fuse-bridge.c
@@ -6352,7 +6352,8 @@ fuse_priv_dump(xlator_t *this)
     if (!this)
         return -1;
 
-    private = this->private;
+   private
+    = this->private;
 
     if (!private)
         return -1;
@@ -6506,7 +6507,8 @@ notify(xlator_t *this, int32_t event, void *data, ...)
     glusterfs_graph_t *graph = NULL;
     struct pollfd pfd = {0};
 
-    private = this->private;
+   private
+    = this->private;
 
     graph = data;
 
@@ -6528,7 +6530,8 @@ notify(xlator_t *this, int32_t event, void *data, ...)
                 (event == GF_EVENT_CHILD_DOWN)) {
                 pthread_mutex_lock(&private->sync_mutex);
                 {
-                    private->event_recvd = 1;
+                   private
+                    ->event_recvd = 1;
                     pthread_cond_broadcast(&private->sync_cond);
                 }
                 pthread_mutex_unlock(&private->sync_mutex);
@@ -6537,16 +6540,18 @@ notify(xlator_t *this, int32_t event, void *data, ...)
             pthread_mutex_lock(&private->sync_mutex);
             {
                 if (!private->fuse_thread_started) {
-                    private->fuse_thread_started = 1;
+                   private
+                    ->fuse_thread_started = 1;
                     start_thread = _gf_true;
                 }
             }
             pthread_mutex_unlock(&private->sync_mutex);
 
             if (start_thread) {
-                private->fuse_thread = GF_CALLOC(private->reader_thread_count,
-                                                 sizeof(pthread_t),
-                                                 gf_fuse_mt_pthread_t);
+               private
+                ->fuse_thread = GF_CALLOC(private->reader_thread_count,
+                                          sizeof(pthread_t),
+                                          gf_fuse_mt_pthread_t);
                 for (i = 0; i < private->reader_thread_count; i++) {
                     ret = gf_thread_create(&private->fuse_thread[i], NULL,
                                            fuse_thread_proc, this, "fuseproc");
@@ -6580,7 +6585,8 @@ notify(xlator_t *this, int32_t event, void *data, ...)
                         if (fuse_get_mount_status(this) != 0) {
                             goto auth_fail_unlock;
                         }
-                        private->mount_finished = _gf_true;
+                       private
+                        ->mount_finished = _gf_true;
                     } else if (pfd.revents) {
                         gf_log(this->name, GF_LOG_ERROR,
                                "mount pipe closed without status");

@xhernandez xhernandez merged commit 5f26bfb into gluster:devel Mar 1, 2023
mohit84 added a commit to mohit84/glusterfs that referenced this pull request Mar 2, 2023
The fuse xlator notify function tries to assign data object to
graph object without checking an event. In case of upcall event data
object represents upcall object so during access of graph object the
process crashed for asan build.

Solution: Access the graph->id only while an event is associated
          specifically to fuse xlator

> Fixes: gluster#3954
> Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
> Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
> (Reviewed on upstream link gluster#4019)

Fixes: gluster#3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
mohit84 added a commit to mohit84/glusterfs that referenced this pull request Mar 2, 2023
The fuse xlator notify function tries to assign data object to graph
object without checking an event. In case of upcall event data object
represents upcall object so during access of graph object the process
crashed for asan build.

Solution: Access the graph->id only while an event is associated
specifically to fuse xlator

> Fixes: gluster#3954
> Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
> Signed-off-by: Mohit Agrawal moagrawa@redhat.com
> (Reviewed on upstream link gluster#4019)

Fixes: gluster#3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
mohit84 added a commit to mohit84/glusterfs that referenced this pull request Mar 2, 2023
The fuse xlator notify function tries to assign data object to graph
object without checking an event. In case of upcall event data object
represents upcall object so during access of graph object the process
crashed for asan build.

Solution: Access the graph->id only while an event is associated
specifically to fuse xlator

> Fixes: gluster#3954
> Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
> Signed-off-by: Mohit Agrawal moagrawa@redhat.com
> (Reviewed on upstream link gluster#4019)

Fixes: gluster#3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
amarts pushed a commit to kadalu/glusterfs that referenced this pull request Mar 20, 2023
…4019)

The fuse xlator notify function tries to assign data object
to graph object without checking an event. In case of upcall
event data object represents upcall object so during access
of graph object the process is crashed for asan build.

Solution: Access the graph->id only while event is associated
          specific to fuse xlator

Fixes: gluster#3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
Shwetha-Acharya pushed a commit that referenced this pull request Mar 30, 2023
The fuse xlator notify function tries to assign data object to graph
object without checking an event. In case of upcall event data object
represents upcall object so during access of graph object the process
crashed for asan build.

Solution: Access the graph->id only while an event is associated
specifically to fuse xlator

> Fixes: #3954
> Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
> Signed-off-by: Mohit Agrawal moagrawa@redhat.com
> (Reviewed on upstream link #4019)

Fixes: #3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
mohit84 added a commit that referenced this pull request Jan 19, 2024
The fuse xlator notify function tries to assign data object to
graph object without checking an event. In case of upcall event data
object represents upcall object so during access of graph object the
process crashed for asan build.

Solution: Access the graph->id only while an event is associated
          specifically to fuse xlator

> Fixes: #3954
> Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf
> Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
> (Reviewed on upstream link #4019)

Fixes: #3954
Change-Id: I6b2869256b26d22163879737dcf163510d1cd8bf

Signed-off-by: Mohit Agrawal <moagrawa@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AddressSanitizer: stack-buffer-overflow in notify at glusterfs/xlators/mount/fuse/src/fuse-bridge.c
3 participants