| From 6d13f69444bd3d4888e43f7756449748f5a98bad Mon Sep 17 00:00:00 2001 |
| From: Al Viro <viro@zeniv.linux.org.uk> |
| Date: Mon, 29 Sep 2014 14:46:30 -0400 |
| Subject: missing data dependency barrier in prepend_name() |
| |
| From: Al Viro <viro@zeniv.linux.org.uk> |
| |
| commit 6d13f69444bd3d4888e43f7756449748f5a98bad upstream. |
| |
| AFAICS, prepend_name() is broken on SMP alpha. Disclaimer: I don't have |
| SMP alpha boxen to reproduce it on. However, it really looks like the race |
| is real. |
| |
| CPU1: d_path() on /mnt/ramfs/<255-character>/foo |
| CPU2: mv /mnt/ramfs/<255-character> /mnt/ramfs/<63-character> |
| |
| CPU2 does d_alloc(), which allocates an external name, stores the name there |
| including terminating NUL, does smp_wmb() and stores its address in |
| dentry->d_name.name. It proceeds to d_add(dentry, NULL) and d_move() |
| old dentry over to that. ->d_name.name value ends up in that dentry. |
| |
| In the meanwhile, CPU1 gets to prepend_name() for that dentry. It fetches |
| ->d_name.name and ->d_name.len; the former ends up pointing to new name |
| (64-byte kmalloc'ed array), the latter - 255 (length of the old name). |
| Nothing to force the ordering there, and normally that would be OK, since we'd |
| run into the terminating NUL and stop. Except that it's alpha, and we'd need |
| a data dependency barrier to guarantee that we see that store of NUL |
| __d_alloc() has done. In a similar situation dentry_cmp() would survive; it |
| does explicit smp_read_barrier_depends() after fetching ->d_name.name. |
| prepend_name() doesn't and it risks walking past the end of kmalloc'ed object |
| and possibly oops due to taking a page fault in kernel mode. |
| |
| Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/dcache.c | 5 +++++ |
| 1 file changed, 5 insertions(+) |
| |
| --- a/fs/dcache.c |
| +++ b/fs/dcache.c |
| @@ -2810,6 +2810,9 @@ static int prepend(char **buffer, int *b |
| * the beginning of the name. The sequence number check at the caller will |
| * retry it again when a d_move() does happen. So any garbage in the buffer |
| * due to mismatched pointer and length will be discarded. |
| + * |
| + * Data dependency barrier is needed to make sure that we see that terminating |
| + * NUL. Alpha strikes again, film at 11... |
| */ |
| static int prepend_name(char **buffer, int *buflen, struct qstr *name) |
| { |
| @@ -2817,6 +2820,8 @@ static int prepend_name(char **buffer, i |
| u32 dlen = ACCESS_ONCE(name->len); |
| char *p; |
| |
| + smp_read_barrier_depends(); |
| + |
| *buflen -= dlen + 1; |
| if (*buflen < 0) |
| return -ENAMETOOLONG; |