[LU-12580] usercopy exposure attempt detected in LL_IOC_LOV_GETSTRIPE ioctl

• Type: Bug
• Resolution: Fixed
• Priority: Critical
• Fix Version/s: Lustre 2.14.0, Lustre 2.12.5
• Affects Version/s: Upstream
• Environment: CentOS 7.6.1810, starting with kernel 3.10.0-957
• Severity: 3
• Assignee: Marco Grossi (Inactive)
• Reporter: Marco Grossi (Inactive)

Description

Software like darshan are still using the old `ioctl` way to gather striping info for a file.

The kernel BUG is easily triggered on a not-PFL striped file, using:

lum->lmm_magic = LOV_USER_MAGIC;
lum->lmm_stripe_count = LOV_MAX_STRIPE_COUNT;

Any `lmm_stripe_count` greater than the actual file's stripe count will trigger the bug.

Kernel side the issue appears to be in `lov_getstripe`: with a positive `lum_size`(line 409), `lmm_size` is set as `lum_size`(line 442) even if `lmm_magic != LOV_MAGIC_COMP_V1`(line 414), while instead the structure is just as big as `lmmk_size`:

404         if (lum.lmm_magic == LOV_USER_MAGIC_V1 ||
405             lum.lmm_magic == LOV_USER_MAGIC_V3)
406                 lum_size = lov_user_md_size(lum.lmm_stripe_count,
407                                             lum.lmm_magic);
408 
409         if (lum_size != 0) {
410                 struct lov_mds_md *comp_md = lmmk;
411 
412                 /* Legacy app (ADIO for instance) treats the layout as V1/V3
413                  * blindly, we'd return a reasonable V1/V3 for them. */
414                 if (lmmk->lmm_magic == LOV_MAGIC_COMP_V1) {
[...]
439                 }
440 
441                 lmm = comp_md;
442                 lmm_size = lum_size;
443         } else {
444                 lmm = lmmk;
445                 lmm_size = lmmk_size;
446         }
447         /**
448          * User specified limited buffer size, usually the buffer is
449          * from ll_lov_setstripe(), and the buffer can only hold basic
450          * layout template info.
451          */
452         if (size == 0 || size > lmm_size)
453                 size = lmm_size;
454         if (copy_to_user(lump, lmm, size))
455                 GOTO(out_free, rc = -EFAULT);

Please find as attachment the kernel trace and a reproducer, to be invoked as:

$ gcc reproducer.c -o reproducer -W -Wall --pedantic
$ ./reproducer <file_path>

Issue Links

• is duplicated by: LU-13596 usercopy: kernel memory exposure attempt detected from ffff98c06ba17d80 (kmalloc-128) (48032 bytes)
• is related to: LU-14316 lfs: using old ioctl(LL_IOC_LOV_GETSTRIPE), use llapi_layout_get_by_path()

Comments

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/38051/
Subject: LU-12580 lov: fix out of bound usercopy
Project: fs/lustre-release
Branch: b2_12
Current Patch Set:
Commit: d4dd52a3a1dea9e6117512889837e245fb983556

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/38050/
Subject: LU-12580 lov: fix typo in lov_comp_md_size
Project: fs/lustre-release
Branch: b2_12
Current Patch Set:
Commit: 13dc723aea39c60d8ed128a312fba1520921c792

[Gerrit Updater]

Minh Diep ([e-mail]) uploaded a new patch: https://review.whamcloud.com/38051
Subject: LU-12580 lov: fix out of bound usercopy
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: b47ff3cd33984e72fc2f59032ad8147074628d28

[Gerrit Updater]

Minh Diep ([e-mail]) uploaded a new patch: https://review.whamcloud.com/38050
Subject: LU-12580 lov: fix typo in lov_comp_md_size
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: 865177bca926efd4f69d4dcaa74624054a2a5aaf

[Peter Jones]

Landed for 2.14

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37469/
Subject: LU-12580 lov: fix out of bound usercopy
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: 2f1beb33144523467b596f4b6fab882b0a839187

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/37493/
Subject: LU-12580 lov: fix typo in lov_comp_md_size
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: d41716533682ed88b8a77654f9b5b050ef5c672c

[Gerrit Updater]

Li Dongyang ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37493
Subject: LU-12580 lov: fix typo in lov_comp_md_size
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: a65fa5d21bb8776735f8d7cf3516aed0db94e64c

[Gerrit Updater]

Li Dongyang ([e-mail]) uploaded a new patch: https://review.whamcloud.com/37469
Subject: LU-12580 lov: fix out of bound usercopy
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: 93df0eba99fbc22af41d11003ba3564394979b29

[Marco Grossi (Inactive)]

Sorry for the confusion; will start by clarifying the first question.

Considering the "struct lov_user_md" passed to the ioctl invocation, it can be either "lov_user_md_v1" or "lov_user_md_v3":

#define lov_user_md lov_user_md_v1

struct lov_user_md_v1 {
        __u32 lmm_magic;
        __u32 lmm_pattern;
        struct ost_id lmm_oi;
        __u32 lmm_stripe_size;
        __u16 lmm_stripe_count;
        union {
                __u16 lmm_stripe_offset;
                __u16 lmm_layout_gen;
        };
        struct lov_user_ost_data_v1 lmm_objects[0];
} __attribute__((packed,  __may_alias__));

struct lov_user_md_v3 {
        __u32 lmm_magic;
        __u32 lmm_pattern;
        struct ost_id lmm_oi;
        __u32 lmm_stripe_size;
        __u16 lmm_stripe_count;
        union {
                __u16 lmm_stripe_offset;
                __u16 lmm_layout_gen;
        };
        char  lmm_pool_name[LOV_MAXPOOLNAME + 1];
        struct lov_user_ost_data_v1 lmm_objects[0];
} __attribute__((packed));

Given that "lmm_stripe_offset" and "lmm_layout_gen" share a union, only one can be returned.

Now, considering a case of "lsm_magic" either of LOV_MAGIC_V1 or LOV_MAGIC_V3, "lov_lsm_pack_v1v3" is the one invoked. In this function, "lmm_layout_gen" is the one chosen to be returned:

lmmv1->lmm_layout_gen = cpu_to_le16(lsm->lsm_layout_gen);

The fact is that majority of codes are expecting to get "lmm_stripe_offset", like ADIO:

    lumlen = sizeof(struct lov_user_md) +
            MAX_LOV_UUID_COUNT * sizeof(struct lov_user_ost_data);
    lum = (struct lov_user_md *)ADIOI_Calloc(1,lumlen);

[...]

    memset(lum, 0, lumlen);
    lum->lmm_magic = LOV_USER_MAGIC;
    err = ioctl(fd->fd_sys, LL_IOC_LOV_GETSTRIPE, (void *)lum);
    if (!err) {

[...]

        fd->hints->fs_hints.lustre.start_iodevice = lum->lmm_stripe_offset;
        sprintf(value, "%d", lum->lmm_stripe_offset);
        ADIOI_Info_set(fd->info, "romio_lustre_start_iodevice", value);

Semantically, which one between "lmm_stripe_offset" and "lmm_layout_gen" should be the correct one to return?