ceph: only use d_name directly when parent is locked
Ben reported tripping the BUG_ON in create_request_message during some performance testing. Analysis of the vmcore showed that the length of the r_dentry->d_name string changed after we allocated the buffer, but before we encoded it. build_dentry_path returns pointers to d_name in the common case of non-snapped dentries, but this optimization isn't safe unless the parent directory is locked. When it isn't, have the code make a copy of the d_name while holding the d_lock. Cc: stable@vger.kernel.org Reported-by: Ben England <bengland@redhat.com> Signed-off-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: "Yan, Zheng" <zyan@redhat.com> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
This commit is contained in:
parent
085b775580
commit
1bcb344086
|
@ -2161,10 +2161,39 @@ retry:
|
||||||
return path;
|
return path;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Duplicate the dentry->d_name.name safely */
|
||||||
|
static int clone_dentry_name(struct dentry *dentry, const char **ppath,
|
||||||
|
int *ppathlen)
|
||||||
|
{
|
||||||
|
u32 len;
|
||||||
|
char *name;
|
||||||
|
|
||||||
|
retry:
|
||||||
|
len = READ_ONCE(dentry->d_name.len);
|
||||||
|
name = kmalloc(len + 1, GFP_NOFS);
|
||||||
|
if (!name)
|
||||||
|
return -ENOMEM;
|
||||||
|
|
||||||
|
spin_lock(&dentry->d_lock);
|
||||||
|
if (dentry->d_name.len != len) {
|
||||||
|
spin_unlock(&dentry->d_lock);
|
||||||
|
kfree(name);
|
||||||
|
goto retry;
|
||||||
|
}
|
||||||
|
memcpy(name, dentry->d_name.name, len);
|
||||||
|
spin_unlock(&dentry->d_lock);
|
||||||
|
|
||||||
|
name[len] = '\0';
|
||||||
|
*ppath = name;
|
||||||
|
*ppathlen = len;
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
static int build_dentry_path(struct dentry *dentry, struct inode *dir,
|
static int build_dentry_path(struct dentry *dentry, struct inode *dir,
|
||||||
const char **ppath, int *ppathlen, u64 *pino,
|
const char **ppath, int *ppathlen, u64 *pino,
|
||||||
int *pfreepath)
|
bool *pfreepath, bool parent_locked)
|
||||||
{
|
{
|
||||||
|
int ret;
|
||||||
char *path;
|
char *path;
|
||||||
|
|
||||||
rcu_read_lock();
|
rcu_read_lock();
|
||||||
|
@ -2173,8 +2202,15 @@ static int build_dentry_path(struct dentry *dentry, struct inode *dir,
|
||||||
if (dir && ceph_snap(dir) == CEPH_NOSNAP) {
|
if (dir && ceph_snap(dir) == CEPH_NOSNAP) {
|
||||||
*pino = ceph_ino(dir);
|
*pino = ceph_ino(dir);
|
||||||
rcu_read_unlock();
|
rcu_read_unlock();
|
||||||
*ppath = dentry->d_name.name;
|
if (parent_locked) {
|
||||||
*ppathlen = dentry->d_name.len;
|
*ppath = dentry->d_name.name;
|
||||||
|
*ppathlen = dentry->d_name.len;
|
||||||
|
} else {
|
||||||
|
ret = clone_dentry_name(dentry, ppath, ppathlen);
|
||||||
|
if (ret)
|
||||||
|
return ret;
|
||||||
|
*pfreepath = true;
|
||||||
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
rcu_read_unlock();
|
rcu_read_unlock();
|
||||||
|
@ -2182,13 +2218,13 @@ static int build_dentry_path(struct dentry *dentry, struct inode *dir,
|
||||||
if (IS_ERR(path))
|
if (IS_ERR(path))
|
||||||
return PTR_ERR(path);
|
return PTR_ERR(path);
|
||||||
*ppath = path;
|
*ppath = path;
|
||||||
*pfreepath = 1;
|
*pfreepath = true;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
static int build_inode_path(struct inode *inode,
|
static int build_inode_path(struct inode *inode,
|
||||||
const char **ppath, int *ppathlen, u64 *pino,
|
const char **ppath, int *ppathlen, u64 *pino,
|
||||||
int *pfreepath)
|
bool *pfreepath)
|
||||||
{
|
{
|
||||||
struct dentry *dentry;
|
struct dentry *dentry;
|
||||||
char *path;
|
char *path;
|
||||||
|
@ -2204,7 +2240,7 @@ static int build_inode_path(struct inode *inode,
|
||||||
if (IS_ERR(path))
|
if (IS_ERR(path))
|
||||||
return PTR_ERR(path);
|
return PTR_ERR(path);
|
||||||
*ppath = path;
|
*ppath = path;
|
||||||
*pfreepath = 1;
|
*pfreepath = true;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -2215,7 +2251,7 @@ static int build_inode_path(struct inode *inode,
|
||||||
static int set_request_path_attr(struct inode *rinode, struct dentry *rdentry,
|
static int set_request_path_attr(struct inode *rinode, struct dentry *rdentry,
|
||||||
struct inode *rdiri, const char *rpath,
|
struct inode *rdiri, const char *rpath,
|
||||||
u64 rino, const char **ppath, int *pathlen,
|
u64 rino, const char **ppath, int *pathlen,
|
||||||
u64 *ino, int *freepath)
|
u64 *ino, bool *freepath, bool parent_locked)
|
||||||
{
|
{
|
||||||
int r = 0;
|
int r = 0;
|
||||||
|
|
||||||
|
@ -2225,7 +2261,7 @@ static int set_request_path_attr(struct inode *rinode, struct dentry *rdentry,
|
||||||
ceph_snap(rinode));
|
ceph_snap(rinode));
|
||||||
} else if (rdentry) {
|
} else if (rdentry) {
|
||||||
r = build_dentry_path(rdentry, rdiri, ppath, pathlen, ino,
|
r = build_dentry_path(rdentry, rdiri, ppath, pathlen, ino,
|
||||||
freepath);
|
freepath, parent_locked);
|
||||||
dout(" dentry %p %llx/%.*s\n", rdentry, *ino, *pathlen,
|
dout(" dentry %p %llx/%.*s\n", rdentry, *ino, *pathlen,
|
||||||
*ppath);
|
*ppath);
|
||||||
} else if (rpath || rino) {
|
} else if (rpath || rino) {
|
||||||
|
@ -2251,7 +2287,7 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
|
||||||
const char *path2 = NULL;
|
const char *path2 = NULL;
|
||||||
u64 ino1 = 0, ino2 = 0;
|
u64 ino1 = 0, ino2 = 0;
|
||||||
int pathlen1 = 0, pathlen2 = 0;
|
int pathlen1 = 0, pathlen2 = 0;
|
||||||
int freepath1 = 0, freepath2 = 0;
|
bool freepath1 = false, freepath2 = false;
|
||||||
int len;
|
int len;
|
||||||
u16 releases;
|
u16 releases;
|
||||||
void *p, *end;
|
void *p, *end;
|
||||||
|
@ -2259,16 +2295,19 @@ static struct ceph_msg *create_request_message(struct ceph_mds_client *mdsc,
|
||||||
|
|
||||||
ret = set_request_path_attr(req->r_inode, req->r_dentry,
|
ret = set_request_path_attr(req->r_inode, req->r_dentry,
|
||||||
req->r_parent, req->r_path1, req->r_ino1.ino,
|
req->r_parent, req->r_path1, req->r_ino1.ino,
|
||||||
&path1, &pathlen1, &ino1, &freepath1);
|
&path1, &pathlen1, &ino1, &freepath1,
|
||||||
|
test_bit(CEPH_MDS_R_PARENT_LOCKED,
|
||||||
|
&req->r_req_flags));
|
||||||
if (ret < 0) {
|
if (ret < 0) {
|
||||||
msg = ERR_PTR(ret);
|
msg = ERR_PTR(ret);
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* If r_old_dentry is set, then assume that its parent is locked */
|
||||||
ret = set_request_path_attr(NULL, req->r_old_dentry,
|
ret = set_request_path_attr(NULL, req->r_old_dentry,
|
||||||
req->r_old_dentry_dir,
|
req->r_old_dentry_dir,
|
||||||
req->r_path2, req->r_ino2.ino,
|
req->r_path2, req->r_ino2.ino,
|
||||||
&path2, &pathlen2, &ino2, &freepath2);
|
&path2, &pathlen2, &ino2, &freepath2, true);
|
||||||
if (ret < 0) {
|
if (ret < 0) {
|
||||||
msg = ERR_PTR(ret);
|
msg = ERR_PTR(ret);
|
||||||
goto out_free1;
|
goto out_free1;
|
||||||
|
|
Loading…
Reference in New Issue