]>
Commit | Line | Data |
---|---|---|
cc4eb9ca GKH |
1 | From 1bcb344086f3ecf8d6705f6d708441baa823beb3 Mon Sep 17 00:00:00 2001 |
2 | From: Jeff Layton <jlayton@kernel.org> | |
3 | Date: Mon, 15 Apr 2019 12:00:42 -0400 | |
4 | Subject: ceph: only use d_name directly when parent is locked | |
5 | ||
6 | From: Jeff Layton <jlayton@kernel.org> | |
7 | ||
8 | commit 1bcb344086f3ecf8d6705f6d708441baa823beb3 upstream. | |
9 | ||
10 | Ben reported tripping the BUG_ON in create_request_message during some | |
11 | performance testing. Analysis of the vmcore showed that the length of | |
12 | the r_dentry->d_name string changed after we allocated the buffer, but | |
13 | before we encoded it. | |
14 | ||
15 | build_dentry_path returns pointers to d_name in the common case of | |
16 | non-snapped dentries, but this optimization isn't safe unless the parent | |
17 | directory is locked. When it isn't, have the code make a copy of the | |
18 | d_name while holding the d_lock. | |
19 | ||
20 | Cc: stable@vger.kernel.org | |
21 | Reported-by: Ben England <bengland@redhat.com> | |
22 | Signed-off-by: Jeff Layton <jlayton@kernel.org> | |
23 | Reviewed-by: "Yan, Zheng" <zyan@redhat.com> | |
24 | Signed-off-by: Ilya Dryomov <idryomov@gmail.com> | |
25 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
26 | ||
27 | --- | |
28 | fs/ceph/mds_client.c | 61 +++++++++++++++++++++++++++++++++++++++++---------- | |
29 | 1 file changed, 50 insertions(+), 11 deletions(-) | |
30 | ||
31 | --- a/fs/ceph/mds_client.c | |
32 | +++ b/fs/ceph/mds_client.c | |
33 | @@ -1958,10 +1958,39 @@ retry: | |
34 | return path; | |
35 | } | |
36 | ||
37 | +/* Duplicate the dentry->d_name.name safely */ | |
38 | +static int clone_dentry_name(struct dentry *dentry, const char **ppath, | |
39 | + int *ppathlen) | |
40 | +{ | |
41 | + u32 len; | |
42 | + char *name; | |
43 | + | |
44 | +retry: | |
45 | + len = READ_ONCE(dentry->d_name.len); | |
46 | + name = kmalloc(len + 1, GFP_NOFS); | |
47 | + if (!name) | |
48 | + return -ENOMEM; | |
49 | + | |
50 | + spin_lock(&dentry->d_lock); | |
51 | + if (dentry->d_name.len != len) { | |
52 | + spin_unlock(&dentry->d_lock); | |
53 | + kfree(name); | |
54 | + goto retry; | |
55 | + } | |
56 | + memcpy(name, dentry->d_name.name, len); | |
57 | + spin_unlock(&dentry->d_lock); | |
58 | + | |
59 | + name[len] = '\0'; | |
60 | + *ppath = name; | |
61 | + *ppathlen = len; | |
62 | + return 0; | |
63 | +} | |
64 | + | |
65 | static int build_dentry_path(struct dentry *dentry, struct inode *dir, | |
66 | const char **ppath, int *ppathlen, u64 *pino, | |
67 | - int *pfreepath) | |
68 | + bool *pfreepath, bool parent_locked) | |
69 | { | |
70 | + int ret; | |
71 | char *path; | |
72 | ||
73 | rcu_read_lock(); | |
74 | @@ -1970,8 +1999,15 @@ static int build_dentry_path(struct dent | |
75 | if (dir && ceph_snap(dir) == CEPH_NOSNAP) { | |
76 | *pino = ceph_ino(dir); | |
77 | rcu_read_unlock(); | |
78 | - *ppath = dentry->d_name.name; | |
79 | - *ppathlen = dentry->d_name.len; | |
80 | + if (parent_locked) { | |
81 | + *ppath = dentry->d_name.name; | |
82 | + *ppathlen = dentry->d_name.len; | |
83 | + } else { | |
84 | + ret = clone_dentry_name(dentry, ppath, ppathlen); | |
85 | + if (ret) | |
86 | + return ret; | |
87 | + *pfreepath = true; | |
88 | + } | |
89 | return 0; | |
90 | } | |
91 | rcu_read_unlock(); | |
92 | @@ -1979,13 +2015,13 @@ static int build_dentry_path(struct dent | |
93 | if (IS_ERR(path)) | |
94 | return PTR_ERR(path); | |
95 | *ppath = path; | |
96 | - *pfreepath = 1; | |
97 | + *pfreepath = true; | |
98 | return 0; | |
99 | } | |
100 | ||
101 | static int build_inode_path(struct inode *inode, | |
102 | const char **ppath, int *ppathlen, u64 *pino, | |
103 | - int *pfreepath) | |
104 | + bool *pfreepath) | |
105 | { | |
106 | struct dentry *dentry; | |
107 | char *path; | |
108 | @@ -2001,7 +2037,7 @@ static int build_inode_path(struct inode | |
109 | if (IS_ERR(path)) | |
110 | return PTR_ERR(path); | |
111 | *ppath = path; | |
112 | - *pfreepath = 1; | |
113 | + *pfreepath = true; | |
114 | return 0; | |
115 | } | |
116 | ||
117 | @@ -2012,7 +2048,7 @@ static int build_inode_path(struct inode | |
118 | static int set_request_path_attr(struct inode *rinode, struct dentry *rdentry, | |
119 | struct inode *rdiri, const char *rpath, | |
120 | u64 rino, const char **ppath, int *pathlen, | |
121 | - u64 *ino, int *freepath) | |
122 | + u64 *ino, bool *freepath, bool parent_locked) | |
123 | { | |
124 | int r = 0; | |
125 | ||
126 | @@ -2022,7 +2058,7 @@ static int set_request_path_attr(struct | |
127 | ceph_snap(rinode)); | |
128 | } else if (rdentry) { | |
129 | r = build_dentry_path(rdentry, rdiri, ppath, pathlen, ino, | |
130 | - freepath); | |
131 | + freepath, parent_locked); | |
132 | dout(" dentry %p %llx/%.*s\n", rdentry, *ino, *pathlen, | |
133 | *ppath); | |
134 | } else if (rpath || rino) { | |
135 | @@ -2048,7 +2084,7 @@ static struct ceph_msg *create_request_m | |
136 | const char *path2 = NULL; | |
137 | u64 ino1 = 0, ino2 = 0; | |
138 | int pathlen1 = 0, pathlen2 = 0; | |
139 | - int freepath1 = 0, freepath2 = 0; | |
140 | + bool freepath1 = false, freepath2 = false; | |
141 | int len; | |
142 | u16 releases; | |
143 | void *p, *end; | |
144 | @@ -2056,16 +2092,19 @@ static struct ceph_msg *create_request_m | |
145 | ||
146 | ret = set_request_path_attr(req->r_inode, req->r_dentry, | |
147 | req->r_parent, req->r_path1, req->r_ino1.ino, | |
148 | - &path1, &pathlen1, &ino1, &freepath1); | |
149 | + &path1, &pathlen1, &ino1, &freepath1, | |
150 | + test_bit(CEPH_MDS_R_PARENT_LOCKED, | |
151 | + &req->r_req_flags)); | |
152 | if (ret < 0) { | |
153 | msg = ERR_PTR(ret); | |
154 | goto out; | |
155 | } | |
156 | ||
157 | + /* If r_old_dentry is set, then assume that its parent is locked */ | |
158 | ret = set_request_path_attr(NULL, req->r_old_dentry, | |
159 | req->r_old_dentry_dir, | |
160 | req->r_path2, req->r_ino2.ino, | |
161 | - &path2, &pathlen2, &ino2, &freepath2); | |
162 | + &path2, &pathlen2, &ino2, &freepath2, true); | |
163 | if (ret < 0) { | |
164 | msg = ERR_PTR(ret); | |
165 | goto out_free1; |