Adds a flag IN_FULL_PATHS which causes inotify
to return the full path instead of only a filename.
Includes a permissions check on IN_MOVE_SELF to prevent
exposing paths if the user does not have permission to view
the new path.
Signed-off-by: Oliver Ford <[email protected]>
---
fs/notify/inotify/inotify_fsnotify.c | 55 ++++++++++++++++++++++------
fs/notify/inotify/inotify_user.c | 19 +++++++++-
include/linux/inotify.h | 2 +-
include/uapi/linux/inotify.h | 1 +
4 files changed, 63 insertions(+), 14 deletions(-)
diff --git a/fs/notify/inotify/inotify_fsnotify.c b/fs/notify/inotify/inotify_fsnotify.c
index 49cfe2ae6d23..6334d1d6d5f5 100644
--- a/fs/notify/inotify/inotify_fsnotify.c
+++ b/fs/notify/inotify/inotify_fsnotify.c
@@ -64,12 +64,39 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
struct inotify_event_info *event;
struct fsnotify_event *fsn_event;
struct fsnotify_group *group = inode_mark->group;
+ struct dentry *en = NULL;
int ret;
int len = 0;
int alloc_len = sizeof(struct inotify_event_info);
struct mem_cgroup *old_memcg;
-
- if (name) {
+ char *path_buf, *path_bufp = NULL;
+ bool found_full_path = false;
+
+ if (inode_mark->mask & IN_FULL_PATHS && inode) {
+ mask |= IN_FULL_PATHS;
+ en = d_find_any_alias(inode);
+ if (en)
+ found_full_path = true;
+ else if (dir)
+ en = d_find_any_alias(dir);
+
+ if (en) {
+ path_buf = kmalloc(PATH_MAX, GFP_KERNEL);
+ if (unlikely(!path_buf))
+ goto oom;
+
+ path_bufp = dentry_path_raw(en, path_buf, PATH_MAX);
+ len = strlen(path_bufp);
+ alloc_len += len + 1;
+
+ if (!found_full_path) {
+ *(path_bufp + len) = '/';
+ strcat(path_bufp + len + 1, name->name);
+ len += name->len + 1;
+ alloc_len += name->len + 1;
+ }
+ }
+ } else if (name) {
len = name->len;
alloc_len += len + 1;
}
@@ -89,14 +116,8 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
event = kmalloc(alloc_len, GFP_KERNEL_ACCOUNT | __GFP_RETRY_MAYFAIL);
set_active_memcg(old_memcg);
- if (unlikely(!event)) {
- /*
- * Treat lost event due to ENOMEM the same way as queue
- * overflow to let userspace know event was lost.
- */
- fsnotify_queue_overflow(group);
- return -ENOMEM;
- }
+ if (unlikely(!event))
+ goto oom;
/*
* We now report FS_ISDIR flag with MOVE_SELF and DELETE_SELF events
@@ -113,8 +134,13 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
event->wd = i_mark->wd;
event->sync_cookie = cookie;
event->name_len = len;
- if (len)
+
+ if (path_bufp) {
+ strcpy(event->name, path_bufp);
+ kfree(path_buf);
+ } else if (len) {
strcpy(event->name, name->name);
+ }
ret = fsnotify_add_event(group, fsn_event, inotify_merge);
if (ret) {
@@ -126,6 +152,13 @@ int inotify_handle_inode_event(struct fsnotify_mark *inode_mark, u32 mask,
fsnotify_destroy_mark(inode_mark, group);
return 0;
+oom:
+ /*
+ * Treat lost event due to ENOMEM the same way as queue
+ * overflow to let userspace know event was lost.
+ */
+ fsnotify_queue_overflow(group);
+ return -ENOMEM;
}
static void inotify_freeing_mark(struct fsnotify_mark *fsn_mark, struct fsnotify_group *group)
diff --git a/fs/notify/inotify/inotify_user.c b/fs/notify/inotify/inotify_user.c
index ed42a189faa2..2a0ad59250ce 100644
--- a/fs/notify/inotify/inotify_user.c
+++ b/fs/notify/inotify/inotify_user.c
@@ -57,6 +57,7 @@ struct kmem_cache *inotify_inode_mark_cachep __read_mostly;
static long it_zero = 0;
static long it_int_max = INT_MAX;
+static struct inotify_inode_mark *inotify_idr_find(struct fsnotify_group *, int);
static struct ctl_table inotify_table[] = {
{
@@ -110,7 +111,7 @@ static inline __u32 inotify_arg_to_mask(struct inode *inode, u32 arg)
mask |= FS_EVENT_ON_CHILD;
/* mask off the flags used to open the fd */
- mask |= (arg & INOTIFY_USER_MASK);
+ mask |= (arg & (INOTIFY_USER_MASK | IN_FULL_PATHS));
return mask;
}
@@ -203,6 +204,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
{
struct inotify_event inotify_event;
struct inotify_event_info *event;
+ struct path event_path;
+ struct inotify_inode_mark *i_mark;
size_t event_size = sizeof(struct inotify_event);
size_t name_len;
size_t pad_name_len;
@@ -210,6 +213,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
pr_debug("%s: group=%p event=%p\n", __func__, group, fsn_event);
event = INOTIFY_E(fsn_event);
+ /* ensure caller has access to view the full path */
+ if (event->mask & IN_FULL_PATHS && event->mask & IN_MOVE_SELF &&
+ kern_path(event->name, 0, &event_path)) {
+ i_mark = inotify_idr_find(group, event->wd);
+ if (likely(i_mark)) {
+ fsnotify_destroy_mark(&i_mark->fsn_mark, group);
+ /* match ref taken by inotify_idr_find */
+ fsnotify_put_mark(&i_mark->fsn_mark);
+ }
+ return -EACCES;
+ }
+
name_len = event->name_len;
/*
* round up name length so it is a multiple of event_size
@@ -860,7 +875,7 @@ static int __init inotify_user_setup(void)
BUILD_BUG_ON(IN_IGNORED != FS_IN_IGNORED);
BUILD_BUG_ON(IN_ISDIR != FS_ISDIR);
- BUILD_BUG_ON(HWEIGHT32(ALL_INOTIFY_BITS) != 22);
+ BUILD_BUG_ON(HWEIGHT32(ALL_INOTIFY_BITS) != 23);
inotify_inode_mark_cachep = KMEM_CACHE(inotify_inode_mark,
SLAB_PANIC|SLAB_ACCOUNT);
diff --git a/include/linux/inotify.h b/include/linux/inotify.h
index 8d20caa1b268..11db0541cff5 100644
--- a/include/linux/inotify.h
+++ b/include/linux/inotify.h
@@ -15,6 +15,6 @@
IN_DELETE_SELF | IN_MOVE_SELF | IN_UNMOUNT | \
IN_Q_OVERFLOW | IN_IGNORED | IN_ONLYDIR | \
IN_DONT_FOLLOW | IN_EXCL_UNLINK | IN_MASK_ADD | \
- IN_MASK_CREATE | IN_ISDIR | IN_ONESHOT)
+ IN_MASK_CREATE | IN_ISDIR | IN_ONESHOT | IN_FULL_PATHS)
#endif /* _LINUX_INOTIFY_H */
diff --git a/include/uapi/linux/inotify.h b/include/uapi/linux/inotify.h
index 884b4846b630..d95e05aa9bd6 100644
--- a/include/uapi/linux/inotify.h
+++ b/include/uapi/linux/inotify.h
@@ -50,6 +50,7 @@ struct inotify_event {
#define IN_MOVE (IN_MOVED_FROM | IN_MOVED_TO) /* moves */
/* special flags */
+#define IN_FULL_PATHS 0x00800000 /* return the absolute path in the name */
#define IN_ONLYDIR 0x01000000 /* only watch the path if it is a directory */
#define IN_DONT_FOLLOW 0x02000000 /* don't follow a sym link */
#define IN_EXCL_UNLINK 0x04000000 /* exclude events on unlinked objects */
--
2.35.1
Hi Oliver,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on jack-fs/fsnotify]
[also build test ERROR on linus/master v5.19-rc1 next-20220606]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/intel-lab-lkp/linux/commits/Oliver-Ford/fs-inotify-Add-full-paths-option-to-inotify/20220607-064615
base: https://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git fsnotify
config: hexagon-randconfig-r018-20220607 (https://download.01.org/0day-ci/archive/20220607/[email protected]/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project b92436efcb7813fc481b30f2593a4907568d917a)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/67d0b1ab6f9129e4902f90506f2ab045ddbae43f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Oliver-Ford/fs-inotify-Add-full-paths-option-to-inotify/20220607-064615
git checkout 67d0b1ab6f9129e4902f90506f2ab045ddbae43f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash fs/notify/inotify/
If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <[email protected]>
All error/warnings (new ones prefixed by >>):
>> fs/notify/inotify/inotify_user.c:219:12: error: call to undeclared function 'inotify_idr_find'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
i_mark = inotify_idr_find(group, event->wd);
^
>> fs/notify/inotify/inotify_user.c:219:10: warning: incompatible integer to pointer conversion assigning to 'struct inotify_inode_mark *' from 'int' [-Wint-conversion]
i_mark = inotify_idr_find(group, event->wd);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> fs/notify/inotify/inotify_user.c:451:35: error: static declaration of 'inotify_idr_find' follows non-static declaration
static struct inotify_inode_mark *inotify_idr_find(struct fsnotify_group *group,
^
fs/notify/inotify/inotify_user.c:219:12: note: previous implicit declaration is here
i_mark = inotify_idr_find(group, event->wd);
^
1 warning and 2 errors generated.
vim +/inotify_idr_find +219 fs/notify/inotify/inotify_user.c
194
195 /*
196 * Copy an event to user space, returning how much we copied.
197 *
198 * We already checked that the event size is smaller than the
199 * buffer we had in "get_one_event()" above.
200 */
201 static ssize_t copy_event_to_user(struct fsnotify_group *group,
202 struct fsnotify_event *fsn_event,
203 char __user *buf)
204 {
205 struct inotify_event inotify_event;
206 struct inotify_event_info *event;
207 struct path event_path;
208 struct inotify_inode_mark *i_mark;
209 size_t event_size = sizeof(struct inotify_event);
210 size_t name_len;
211 size_t pad_name_len;
212
213 pr_debug("%s: group=%p event=%p\n", __func__, group, fsn_event);
214
215 event = INOTIFY_E(fsn_event);
216 /* ensure caller has access to view the full path */
217 if (event->mask & IN_FULL_PATHS && event->mask & IN_MOVE_SELF &&
218 kern_path(event->name, 0, &event_path)) {
> 219 i_mark = inotify_idr_find(group, event->wd);
220 if (likely(i_mark)) {
221 fsnotify_destroy_mark(&i_mark->fsn_mark, group);
222 /* match ref taken by inotify_idr_find */
223 fsnotify_put_mark(&i_mark->fsn_mark);
224 }
225 return -EACCES;
226 }
227
228 name_len = event->name_len;
229 /*
230 * round up name length so it is a multiple of event_size
231 * plus an extra byte for the terminating '\0'.
232 */
233 pad_name_len = round_event_name_len(fsn_event);
234 inotify_event.len = pad_name_len;
235 inotify_event.mask = inotify_mask_to_arg(event->mask);
236 inotify_event.wd = event->wd;
237 inotify_event.cookie = event->sync_cookie;
238
239 /* send the main event */
240 if (copy_to_user(buf, &inotify_event, event_size))
241 return -EFAULT;
242
243 buf += event_size;
244
245 /*
246 * fsnotify only stores the pathname, so here we have to send the pathname
247 * and then pad that pathname out to a multiple of sizeof(inotify_event)
248 * with zeros.
249 */
250 if (pad_name_len) {
251 /* copy the path name */
252 if (copy_to_user(buf, event->name, name_len))
253 return -EFAULT;
254 buf += name_len;
255
256 /* fill userspace with 0's */
257 if (clear_user(buf, pad_name_len - name_len))
258 return -EFAULT;
259 event_size += pad_name_len;
260 }
261
262 return event_size;
263 }
264
--
0-DAY CI Kernel Test Service
https://01.org/lkp
On Mon, Jun 06, 2022 at 11:42:41PM +0100, Oliver Ford wrote:
> @@ -203,6 +204,8 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
> {
> struct inotify_event inotify_event;
> struct inotify_event_info *event;
> + struct path event_path;
> + struct inotify_inode_mark *i_mark;
> size_t event_size = sizeof(struct inotify_event);
> size_t name_len;
> size_t pad_name_len;
> @@ -210,6 +213,18 @@ static ssize_t copy_event_to_user(struct fsnotify_group *group,
> pr_debug("%s: group=%p event=%p\n", __func__, group, fsn_event);
>
> event = INOTIFY_E(fsn_event);
> + /* ensure caller has access to view the full path */
> + if (event->mask & IN_FULL_PATHS && event->mask & IN_MOVE_SELF &&
> + kern_path(event->name, 0, &event_path)) {
> + i_mark = inotify_idr_find(group, event->wd);
> + if (likely(i_mark)) {
> + fsnotify_destroy_mark(&i_mark->fsn_mark, group);
> + /* match ref taken by inotify_idr_find */
> + fsnotify_put_mark(&i_mark->fsn_mark);
> + }
> + return -EACCES;
> + }
> +
What. The. Hell?
Could you please explain how is it not a massive dentry and mount leak and
just what is being attempted here, anyway?
Incidentally, who said that pathname will be still resolving to whatever
it used to resolve to back when the operation had happened? Or that
it would make any sense for the read(2) caller, while we are at it...
NAKed-by: Al Viro <[email protected]>