Whamcloud - gitweb
LU-12020 llite: make sure name pack atomic 65/34465/2
authorWang Shilong <wshilong@ddn.com>
Tue, 26 Feb 2019 14:38:29 +0000 (22:38 +0800)
committerOleg Drokin <green@whamcloud.com>
Mon, 1 Apr 2019 06:18:51 +0000 (06:18 +0000)
We are trying to access dentry name directly and pass it
down without holding @d_lock, this is racy and possibly
make us trigger assertions:

(mdc_lib.c:137:mdc_pack_name()) ASSERTION( lu_name_is_valid_2(buf, cpy_len) ) failed:

Fix the problem by allocting memory and copy name with @d_lock
held.

Lustre-change: https://review.whamcloud.com/34330
Lustre-commit: f575b6551b2b8690894baeab95d6fe35e57e9418

Change-Id: Iae0066661f42e8fca9358cbedd9cb21828779bbb
Signed-off-by: Wang Shilong <wshilong@ddn.com>
Reviewed-by: Patrick Farrell <pfarrell@whamcloud.com>
Reviewed-by: Gu Zheng <gzheng@ddn.com>
Signed-off-by: Minh Diep <mdiep@whamcloud.com>
Reviewed-on: https://review.whamcloud.com/34465
Tested-by: Jenkins
Tested-by: Maloo <maloo@whamcloud.com>
Reviewed-by: Oleg Drokin <green@whamcloud.com>
lustre/llite/file.c

index a2c7294..7076050 100644 (file)
@@ -498,7 +498,7 @@ static int ll_intent_file_open(struct dentry *de, void *lmm, int lmmsize,
 {
        struct ll_sb_info *sbi = ll_i2sbi(de->d_inode);
        struct dentry *parent = de->d_parent;
-       const char *name = NULL;
+       char *name = NULL;
        int len = 0;
        struct md_op_data *op_data;
        struct ptlrpc_request *req = NULL;
@@ -510,21 +510,41 @@ static int ll_intent_file_open(struct dentry *de, void *lmm, int lmmsize,
 
        /* if server supports open-by-fid, or file name is invalid, don't pack
         * name in open request */
-       if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_OPEN_BY_FID) &&
-           lu_name_is_valid_2(de->d_name.name, de->d_name.len)) {
-               name = de->d_name.name;
+       if (!(exp_connect_flags(sbi->ll_md_exp) & OBD_CONNECT_OPEN_BY_FID)) {
+retry:
                len = de->d_name.len;
+               name = kmalloc(len, GFP_NOFS);
+               if (!name)
+                       RETURN(-ENOMEM);
+               /* race here */
+               spin_lock(&de->d_lock);
+               if (len != de->d_name.len) {
+                       spin_unlock(&de->d_lock);
+                       kfree(name);
+                       goto retry;
+               }
+               memcpy(name, de->d_name.name, len);
+               spin_unlock(&de->d_lock);
+
+               if (!lu_name_is_valid_2(name, len)) {
+                       kfree(name);
+                       name = NULL;
+                       len = 0;
+               }
        }
 
        op_data = ll_prep_md_op_data(NULL, parent->d_inode, de->d_inode,
                                     name, len, 0, LUSTRE_OPC_ANY, NULL);
-       if (IS_ERR(op_data))
+       if (IS_ERR(op_data)) {
+               kfree(name);
                RETURN(PTR_ERR(op_data));
+       }
        op_data->op_data = lmm;
        op_data->op_data_size = lmmsize;
 
        rc = md_intent_lock(sbi->ll_md_exp, op_data, itp, &req,
                            &ll_md_blocking_ast, 0);
+       kfree(name);
        ll_finish_md_op_data(op_data);
        if (rc == -ESTALE) {
                /* reason for keep own exit path - don`t flood log