[LU-6674] struct lov_user_mds_data can be used uninitialized

• Status: Resolved
• Project: Lustre
• Fix Version/s: Lustre 2.8.0
• Type: Bug
• Priority: Critical
• Reporter: Dmitry Eremin (Inactive)
• Assignee: Dmitry Eremin (Inactive)
• Resolution: Fixed
• Severity: 3

Description

In function cb_find_init():3003 requested the lmd structure but it filled partially and struct lov_user_mds_data becomes uninitialized because of following code in ll_dir_ioctl():

                if (cmd == IOC_MDC_GETFILEINFO ||
                    cmd == IOC_MDC_GETFILESTRIPE) {
			filename = ll_getname((const char __user *)arg);
                        if (IS_ERR(filename))
                                RETURN(PTR_ERR(filename));

                        rc = ll_lov_getstripe_ea_info(inode, filename, &lmm,
                                                      &lmmsize, &request);
		} else {
			rc = ll_dir_getstripe(inode, (void **)&lmm, &lmmsize,
					      &request, 0);
		}
[...]
                if (rc < 0) {
                        if (rc == -ENODATA && (cmd == IOC_MDC_GETFILEINFO ||
                                               cmd == LL_IOC_MDC_GETINFO))
                                GOTO(skip_lmm, rc = 0);
                        else
                                GOTO(out_req, rc);
                }

                if (cmd == IOC_MDC_GETFILESTRIPE ||
                    cmd == LL_IOC_LOV_GETSTRIPE) {
			lump = (struct lov_user_md __user *)arg;
                } else {
			struct lov_user_mds_data __user *lmdp;
			lmdp = (struct lov_user_mds_data __user *)arg;
                        lump = &lmdp->lmd_lmm;
                }
		if (copy_to_user(lump, lmm, lmmsize)) {
			if (copy_to_user(lump, lmm, sizeof(*lump)))
                                GOTO(out_req, rc = -EFAULT);
                        rc = -EOVERFLOW;
                }
        skip_lmm:

Comments

[Gerrit Updater]

Dmitry Eremin ([e-mail]) uploaded a new patch: http://review.whamcloud.com/15109
Subject: LU-6674: test: test of uninitilized structure used
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 3f229741c9d5c5f8671fcbe953e19669cf222991

[Dmitry Eremin (Inactive)]

After this patch I got 6 sanity tests failures:

# grep "FAIL " sanity.log 
FAIL 56y (1s)
FAIL 184c (18s)
FAIL 200 (31s)
FAIL 220 (18s)
FAIL 244 (2s)
FAIL 251 (1s)

[Peter Jones]

John

Oleg thought that you might have some input to this one

Peter

[Dmitry Eremin (Inactive)]

This functionality was add by

commit d39b08def6512ee6ae883a0db62cebd808646208
Author: jcl <[e-mail]>
Date:   Wed Feb 20 11:18:23 2013 +0100

    LU-3363 api: HSM import uses new released pattern

    Import creates a released file using new RAID pattern flag
    Import used a new ioctl() to implement the import in the
    client kernel.
    Add a -L|--layout option to lfs getstripe and lfs find

    Signed-off-by: JC Lafoucriere <[e-mail]>
    Change-Id: I39103e6f90bd73dc09cde49086d223d717e41cd2
    Reviewed-on: http://review.whamcloud.com/6536
    Tested-by: Hudson
    Reviewed-by: Andreas Dilger <[e-mail]>
    Reviewed-by: Jinshan Xiong <[e-mail]>
    Tested-by: Maloo <[e-mail]>

It looks this issue was originally.

[Dmitry Eremin (Inactive)]

	/* Request MDS for the stat info if some of these parameters need
	 * to be compared. */
	if (param->fp_obd_uuid || param->fp_mdt_uuid ||
	    param->fp_check_uid || param->fp_check_gid ||
	    param->fp_atime || param->fp_mtime || param->fp_ctime ||
	    param->fp_check_pool || param->fp_check_size ||
	    param->fp_check_stripe_count || param->fp_check_stripe_size ||
	    param->fp_check_layout)
		decision = 0;

	if (param->fp_type != 0 && checked_type == 0)
                decision = 0;

	if (decision == 0) {
		ret = get_lmd_info(path, parent, dir, param->fp_lmd,
				   param->fp_lum_size);

The issue is get_lmd_info() return 0 and nothing in param->fp_lmd->lmd_lmm. Therefore previous data or garbage is used for subsequent comparison:

	if (param->fp_check_layout) {
		__u32 found;

		found = (param->fp_lmd->lmd_lmm.lmm_pattern & param->fp_layout);
		if ((param->fp_lmd->lmd_lmm.lmm_pattern == 0xFFFFFFFF) ||
		    (found && param->fp_exclude_layout) ||
		    (!found && !param->fp_exclude_layout)) {
			decision = -1;
			goto decided;
		}
	}

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch http://review.whamcloud.com/15109/
Subject: LU-6674 utils: fix of uninitilized lmm structure usage
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 486d2797fc0d04e2482ecd4c7100919c55d07277