| Message ID | aa98a3ad00666a6fc0ce411755de4a1a60f5c0cd.1651174324.git.rgb@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fanotify: Allow user space to pass back additional audit info | expand |
On Thu, Apr 28, 2022 at 8:45 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > The user space API for the response variable is __u32. This patch makes > sure that the whole path through the kernel uses __u32 so that there is > no sign extension or truncation of the user space response. > > Suggested-by: Steve Grubb <sgrubb@redhat.com> > Link: https://lore.kernel.org/r/12617626.uLZWGnKmhe@x2 > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > Link: https://lore.kernel.org/r/aa98a3ad00666a6fc0ce411755de4a1a60f5c0cd.1651174324.git.rgb@redhat.com > --- > fs/notify/fanotify/fanotify.h | 2 +- > fs/notify/fanotify/fanotify_user.c | 6 +++--- > 2 files changed, 4 insertions(+), 4 deletions(-) It seems like audit_fanotify()/__audit_fanotify() should also be changed, yes? Granted, in this case it's an unsigned int to u32 conversion so not really all that critical, but if you are going to update the fanotify code you might as well update the audit code as well for the sake of completeness. -- paul-moore.com
On 2022-05-02 20:16, Paul Moore wrote: > On Thu, Apr 28, 2022 at 8:45 PM Richard Guy Briggs <rgb@redhat.com> wrote: > > > > The user space API for the response variable is __u32. This patch makes > > sure that the whole path through the kernel uses __u32 so that there is > > no sign extension or truncation of the user space response. > > > > Suggested-by: Steve Grubb <sgrubb@redhat.com> > > Link: https://lore.kernel.org/r/12617626.uLZWGnKmhe@x2 > > Signed-off-by: Richard Guy Briggs <rgb@redhat.com> > > Link: https://lore.kernel.org/r/aa98a3ad00666a6fc0ce411755de4a1a60f5c0cd.1651174324.git.rgb@redhat.com > > --- > > fs/notify/fanotify/fanotify.h | 2 +- > > fs/notify/fanotify/fanotify_user.c | 6 +++--- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > It seems like audit_fanotify()/__audit_fanotify() should also be > changed, yes? Granted, in this case it's an unsigned int to u32 > conversion so not really all that critical, but if you are going to > update the fanotify code you might as well update the audit code as > well for the sake of completeness. Yes, that was somewhere in the back of my mind but forgot to come back to it. Thanks for catching that. > paul-moore.com - RGB -- Richard Guy Briggs <rgb@redhat.com> Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada IRC: rgb, SunRaycer Voice: +1.647.777.2635, Internal: (81) 32635
diff --git a/fs/notify/fanotify/fanotify.h b/fs/notify/fanotify/fanotify.h index a3d5b751cac5..70acfd497771 100644 --- a/fs/notify/fanotify/fanotify.h +++ b/fs/notify/fanotify/fanotify.h @@ -425,7 +425,7 @@ FANOTIFY_PE(struct fanotify_event *event) struct fanotify_perm_event { struct fanotify_event fae; struct path path; - unsigned short response; /* userspace answer to the event */ + __u32 response; /* userspace answer to the event */ unsigned short state; /* state of the event */ int fd; /* fd we passed to userspace for this event */ }; diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index 9b32b76a9c30..694516470660 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -289,7 +289,7 @@ static int create_fd(struct fsnotify_group *group, struct path *path, */ static void finish_permission_event(struct fsnotify_group *group, struct fanotify_perm_event *event, - unsigned int response) + __u32 response) __releases(&group->notification_lock) { bool destroy = false; @@ -310,9 +310,9 @@ static int process_access_response(struct fsnotify_group *group, { struct fanotify_perm_event *event; int fd = response_struct->fd; - int response = response_struct->response; + __u32 response = response_struct->response; - pr_debug("%s: group=%p fd=%d response=%d\n", __func__, group, + pr_debug("%s: group=%p fd=%d response=%u\n", __func__, group, fd, response); /* * make sure the response is valid, if invalid we do nothing and either
The user space API for the response variable is __u32. This patch makes sure that the whole path through the kernel uses __u32 so that there is no sign extension or truncation of the user space response. Suggested-by: Steve Grubb <sgrubb@redhat.com> Link: https://lore.kernel.org/r/12617626.uLZWGnKmhe@x2 Signed-off-by: Richard Guy Briggs <rgb@redhat.com> Link: https://lore.kernel.org/r/aa98a3ad00666a6fc0ce411755de4a1a60f5c0cd.1651174324.git.rgb@redhat.com --- fs/notify/fanotify/fanotify.h | 2 +- fs/notify/fanotify/fanotify_user.c | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-)