[LU-9513] New static analysis issues in v2_9_57_0-66-gb6d5b5b

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.10.0
• Affects Version/s: Lustre 2.10.0
• Labels: kw
• Severity: 3
• Assignee: Niu Yawei (Inactive)
• Reporter: Dmitry Eremin (Inactive)

Description

Found 1 new static analysis issues in v2_9_57_0-66-gb6d5b5b:

1. Result of function that may return NULL will be dereferenced
   • lustre/utils/liblustreapi_layout.c: in llapi_layout_file_comp_del, Pointer 'comp' returned from call to function '__llapi_layout_cur_comp' at line 1963 may be NULL and will be dereferenced at line 1964.

The list of commits since the previous build v2_9_57_0-45-gcdc7b3b:

b6d5b5b	LU-9415 lfsck: quiet noisy console message
2ef0f3a	LU-9477 tests: check correct handling of dead object
2dd720a	LU-9464 hsm: use OBD_ALLOC_LARGE() for hsm_scan_data array
a003d8d	LU-9119 lnet: Fix deleting peers from YAML
a6d463f	LU-9315 pfl: static analysis issues
49f4bd0	LU-9241 llite: ASSERTION( de->d_op == &ll_d_ops ) failed
e96c70e	LU-9202 lfsck: skip unavailable targets when sync failures
5d32447	LU-9183 llite: remove struct file on stack in ll_setxattr()
fa3598d	LU-9183 ptlrpc: handle changes in struct group_info
1852513	LU-8730 test: Remove duplicate $dev from conf-sanity test_83
434eeba	LU-8589 osd: remove "object" from method function names
c60e949	LU-8359 ldlm: Wrong evict during failover
3800d05	LU-9305 osd-zfs: arc_buf could be non-pagesize aligned
f240769	LU-8670 tests: test_115 Fixes & Improvements
b27652c	LU-6952 mount: Mount options parsing problem
b76a5b1	LU-9425 lnd: Turn on 2 sges by default
5611dbe	LU-7584 tests: clean up sanity test_129
da47e9c	LU-9362 lfs: getstripe print last init-ed component info
e79e711	LU-8998 tests: improve sanity.sh::check_seq_oid()
7a1c0f9	LU-8998 tools: support negative flags
64b2fad	LU-7473 acl: increase ACL entries limitation

Issue Links

• is related to: LU-4629 Issues found by static analysis tools
• is related to: LU-8998 Progressive File Layout (PFL)

Comments

[Peter Jones]

Landed for 2.10

[Gerrit Updater]

Oleg Drokin ([e-mail]) merged in patch https://review.whamcloud.com/27154/
Subject: LU-9513 liblustre: always check return value
Project: fs/lustre-release
Branch: master
Current Patch Set:
Commit: b58fcb7fe4f63830ccc3e6c1747cb1b361d481dd

[Gerrit Updater]

Niu Yawei ([e-mail]) uploaded a new patch: https://review.whamcloud.com/27154
Subject: LU-9513 liblustre: always check return value
Project: fs/lustre-release
Branch: master
Current Patch Set: 1
Commit: bcf5cf8502221dbca1610723b999193f7d91209f

[Dmitry Eremin (Inactive)]

My position is we should write robust code. Even if now this condition is not possible we should check for it to avoid crash or unxpected behavior in the future when something can be changed. We cannot remember all details of all calling conventions especially without comments.

[Zhenyu Xu]

current code can make sure that 1963 returned a non-NULL pointer, while I think we'd add the check, since further code change could possibly introduce NULL pointer.

[Niu Yawei (Inactive)]

        layout = llapi_layout_alloc();
        if (layout == NULL)
                return -1;

        llapi_layout_comp_extent_set(layout, 0, LUSTRE_EOF);
        comp = __llapi_layout_cur_comp(layout);
        comp->llc_id = id;
        comp->llc_flags = flags;

For this particular call to "__llapi_layout_cur_comp(layout)", it's different from other callers (where the 'layout' is passed in from user apps), the 'layout' is constructed internally by the caller itself, so we can guarantee the 'layout' is valid, and the __llapi_layout_cur_comp() won't return a NULL on a valid 'layout'.

I'm wondering why the analysis tool warn about this, is this a shortcoming (or defect) of the tool? Or I missed anything in the code? Bobi, could you double check the code to see if I missed anything which may lead to a NULL being returned?

Dmitry/Andreas, should we add redundant check to make analysis tool happy or just leave it as-is? I'm not quite sure what policy we used for such kind of issues.

[Peter Jones]

Niu

Could you pelase assist with this issue?

Peter