[LU-9513] New static analysis issues in v2_9_57_0-66-gb6d5b5b Created: 16/May/17  Updated: 30/Aug/23  Resolved: 29/May/17

Status: Resolved
Project: Lustre
Component/s: None
Affects Version/s: Lustre 2.10.0
Fix Version/s: Lustre 2.10.0

Type: Bug Priority: Minor
Reporter: Dmitry Eremin (Inactive) Assignee: Niu Yawei (Inactive)
Resolution: Fixed Votes: 0
Labels: kw

Issue Links:
Related
is related to LU-4629 Issues found by static analysis tools Resolved
is related to LU-8998 Progressive File Layout (PFL) Resolved
Severity: 3
Rank (Obsolete): 9223372036854775807

 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


 Comments   
Comment by Peter Jones [ 16/May/17 ]

Niu

Could you pelase assist with this issue?

Peter

Comment by Niu Yawei (Inactive) [ 17/May/17 ]
        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.

Comment by Zhenyu Xu [ 17/May/17 ]

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.

Comment by Dmitry Eremin (Inactive) [ 17/May/17 ]

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.

Comment by Gerrit Updater [ 17/May/17 ]

Niu Yawei (yawei.niu@intel.com) 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

Comment by Gerrit Updater [ 29/May/17 ]

Oleg Drokin (oleg.drokin@intel.com) 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

Comment by Peter Jones [ 29/May/17 ]

Landed for 2.10

Generated at Sat Feb 10 02:26:51 UTC 2024 using Jira 9.4.14#940014-sha1:734e6822bbf0d45eff9af51f82432957f73aa32c.