[LU-12285] Wrong variable scope used in jt_llog_print

• Status: Resolved
• Project: Lustre
• Affects Version/s: Lustre 2.12.2
• Type: Bug
• Priority: Minor
• Reporter: Chris Horn
• Assignee: WC Triage
• Resolution: Duplicate
• Severity: 3

Description

Issue found in b2_12. This issue lead to a sanity failure: sanity: FAIL: test_60aa old llog_print failed

The wrong variable scope is used to assign default values to data.ioc_inlbuf2 and data.ioc_inlbuf3 in jt_llog_print()

int jt_llog_print(int argc, char **argv)
{
        struct obd_ioctl_data data;
        char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
        int rc;

        if (argc != 2 && argc != 4)
                return CMD_HELP;

        memset(&data, 0, sizeof(data));
        data.ioc_dev = cur_device;
        data.ioc_inllen1 = strlen(argv[1]) + 1;
        data.ioc_inlbuf1 = argv[1];
        if (argc == 4) {
                data.ioc_inllen2 = strlen(argv[2]) + 1;
                data.ioc_inlbuf2 = argv[2];
                data.ioc_inllen3 = strlen(argv[3]) + 1;
                data.ioc_inlbuf3 = argv[3];
        } else {
                char from[2] = "1", to[3] = "-1";
                data.ioc_inllen2 = strlen(from) + 1;
                data.ioc_inlbuf2 = from;
                data.ioc_inllen3 = strlen(to) + 1;
                data.ioc_inlbuf3 = to;
        }

Declaring "from" and "to" inside the else block leads to undefined behavior.

For example, with gcc 7.4.1 on sles 15 those buffers contain an empty string after exiting the else block.

int jt_llog_print(int argc, char **argv)
{
        struct obd_ioctl_data data;
        char rawbuf[MAX_IOC_BUFLEN], *buf = rawbuf;
        int rc;

        if (argc != 2 && argc != 4)
                return CMD_HELP;

        memset(&data, 0, sizeof(data));
        data.ioc_dev = cur_device;
        data.ioc_inllen1 = strlen(argv[1]) + 1;
        data.ioc_inlbuf1 = argv[1];
        printf("argc: %d\n", argc);
        if (argc == 4) {
                data.ioc_inllen2 = strlen(argv[2]) + 1;
                data.ioc_inlbuf2 = argv[2];
                data.ioc_inllen3 = strlen(argv[3]) + 1;
                data.ioc_inlbuf3 = argv[3];
        } else {
                char from[2] = "1", to[3] = "-1";
                data.ioc_inllen2 = strlen(from) + 1;
                data.ioc_inlbuf2 = from;
                data.ioc_inllen3 = strlen(to) + 1;
                data.ioc_inlbuf3 = to;
        }
        printf("from: %s to: %s\n", data.ioc_inlbuf2, data.ioc_inlbuf3);

sles15s01:/home/build/lustre-filesystem/lustre/tests # ../utils/lctl --device MGS llog_print lustre-client
argc: 2
from:  to:

Issue Links

• is related to: LU-11566 sanity test_60aa: llog_print_cb()) no...

Comments

[Chris Horn]

Sorry, I didn't check master before opening this ticket. I see the code has changed, so this probably isn't an issue there.

[Gerrit Updater]

Chris Horn ([e-mail]) uploaded a new patch: https://review.whamcloud.com/34846
Subject: LU-12285 utils: Correct variable scope in jt_llog_print
Project: fs/lustre-release
Branch: b2_12
Current Patch Set: 1
Commit: dc309d8900610e0e201569f8be634d4fcad15979

[Andreas Dilger]

The better way to fix this is the backport of patch https://review.whamcloud.com/34850 "LU-11566 utils: fix lctl llog_print for large configs", since that also fixes the issue that "lctl llog_print" doesn't work if the config log is larger than about 200 records.

That patch was meant to be included into 2.12.0 but didn't quite make it, and should have been cherry-picked before now, but was missed for some reason.

[Chris Horn]

Okay, I agree with that. I'll abandon my patch for b2_12 and will take your backport for a spin. This ticket can be closed.

[Andreas Dilger]

What is interesting is that this bug existed for quite a while before it was fixed with the above patch:

commit c5050e412572b00cbe93d8517d2d1f767bebfa92
Author:     phil <phil>
AuthorDate: Wed Dec 3 03:16:26 2003 +0000

    land v0.9.1 on HEAD, in preparation for a 1.0.x branch

[Andreas Dilger]

Using patch https://review.whamcloud.com/34850 to fix this on b2_12.