Fix seq_read dead loop and trigger memory allocation failure.

When dump /proc/xxx/maps, if d_path return error in seq_path, the buffer will be exhaust and trigger dead loop in seq_read. Till kmalloc fails with -ENOMEM.

fs/proc/task_mmu.c +1 -0

@@ -295,8 +295,16 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma, int is_pid)
 	 * special [heap] marker for the heap:
 	 */
 	if (file) {
+		size_t sz;

Quoted 3 lines · Fengwei Yin

When dump /proc/xxx/maps, if d_path return error in seq_path, the buffer will be exhaust and trigger dead loop in seq_read. Till kmalloc fails with -ENOMEM.

*WHAT* d_path error? -ENAMETOOLONG, aka. "you've got too little space"?

Quoted 12 lines

fs/proc/task_mmu.c +6 -1

@@ -295,8 +295,16 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma, int is_pid)
       * special [heap] marker for the heap:
       */
      if (file) {
+             size_t sz;
              seq_pad(m, ' ');
-             seq_path(m, &file->f_path, "\n");
+             /* Save current count. Once seq_path return negtive value,
+              * we need to restore saved count. Otherwise, seq_path will
+              * exhaust the buffer and make seq_read dead loop till
+              * m->buff allocation failure.
+              */

fs/proc/task_mmu.c show_map_vma(struct seq_file *m, struct vm_area_struct *vma, int is_pid) · line 307

Quoted 3 lines

fs/proc/task_mmu.c +3 -0

+             sz = m->count;
+             if (seq_path(m, &file->f_path, "\n") < 0)
+                     m->count = sz;

NAK. No way in hell. Any code playing with m->count that way is broken. Post the reproducer for that infinite loop; then we'll be able to see what triggers an impossible error from d_path(). _That_ is where the bug is, assuming it exists at all.

Quoted 7 lines · Al Viro

Quoted 3 lines · Fengwei Yin

When dump /proc/xxx/maps, if d_path return error in seq_path, the buffer will be exhaust and trigger dead loop in seq_read. Till kmalloc fails with -ENOMEM.

*WHAT* d_path error? -ENAMETOOLONG, aka. "you've got too little space"?

I could check it and get you back. But I suppose it's not this one because it still fails even I have buffer with 4M size.

Quoted 10 lines · Fengwei Yin

Quoted 7 lines · Al Viro

Quoted 3 lines · Fengwei Yin

When dump /proc/xxx/maps, if d_path return error in seq_path, the buffer will be exhaust and trigger dead loop in seq_read. Till kmalloc fails with -ENOMEM.

*WHAT* d_path error? -ENAMETOOLONG, aka. "you've got too little space"?

I could check it and get you back. But I suppose it's not this one because it still fails even I have buffer with 4M size.

Please, do. One thing to watch out for is bogus ->d_dname() return value; instances of ->d_dname() are only allowed to return valid pointers or ERR_PTR(-ENAMETOOLONG), the latter - only if the buffer really is too short (i.e. disappearing on sufficiently large one). That holds in mainline, but that's the most likely vector by which the breakage could be introduced in some out-of-tree code.

Here's the braindump on that bunch (basically, everything in fs/dcache.c from prepend() to dentry_path()):
* their char * arguments are never ERR_PTR(...)
* their char ** arguments never point to ERR_PTR(...) - not on entry
to function and not on return from it, regardless of return value.
* prepend(), prepend_name() and prepend_unreachable() always return
either 0 or -ENAMETOOLONG.
* return value of prepend_path() and path_with_deleted() can only be
0, 1, 2 or -ENAMETOOLONG.
* __d_path() returns NULL, a pointer to string or
ERR_PTR(-ENAMETOOLONG).
* d_absolute_path() returns a pointer to string, ERR_PTR(-ENAMETOOLONG)
or ERR_PTR(-EINVAL), the last one being for the case when its path argument points into an unmounted vfsmount.
* d_path(), __dentry_path(), dentry_path_raw(), dentry_path() and
->d_dname() instances return a pointer to string or ERR_PTR(-ENAMETOOLONG).
* all in-tree instances of ->d_dname() are either simple_dname() or
trivial wrappers for dynamic_dname(), so for mainline it's enough to check those two helpers; out-of-tree code providing ->d_dname() instances needs to be checked, of course.

* given sufficiently large buffer ->d_dname() should succeed.
Persistent ERR_PTR(-ENAMETOOLONG) from it is a bug. Note that use of dynamic_dname() with format that might exceed 64 characters of output is wrong; that's the reason why e.g. mm/shmem.c uses simple_dname() instead. AFAICS, all remaining callers of dynamic_dname() in mainline are guaranteed to stay within its limitations (either <short string>[<unsigned long decimal>] or anon_inode:<short string passed to anon_inode_get{file,fd}>). Out-of-tree code needs to be checked, of course.