-
Bug
-
Resolution: Unresolved
-
Minor
-
Lustre 2.18.0
-
None
-
3
-
9223372036854775807
Ai generated report following:
h1. Summary
lustre: container_of_safe() in the downcast accessors makes every caller a
potential ERR_PTR dereference
Description
A smatch scan of current master (kernel modules only, dd48e6b096) produces
662 reports outside the out-of-scope generated ldiskfs/ tree. *494 of them
– three quarters of the whole scan – are one class*, and they all come from a
single macro.
The ~46 inline accessors that downcast an lu_device or lu_object (or a
cl_, dt_ or md_ layer of one) to its containing per-module
structure – lu2dt_dev(), lu2cl(), mdt_obj(), lu2lod_dev() and
the rest – are built on container_of_safe()
(include/linux/libcfs/libcfs.h:79):
#define container_of_safe(ptr, type, member) \ (IS_ERR_OR_NULL(ptr) ? ERR_CAST(ptr) \ : container_of(ptr, type, member))
so each of them returns a NULL or an ERR_PTR argument unchanged instead
of doing the subtraction. A downcast that can hand back an error pointer makes
every caller a potential error-pointer dereference, which is exactly what
smatch reports: *439 "dereferencing possible ERR_PTR()" errors plus 39 "passing
zero to ERR_CAST" warnings*, spread across essentially every server and client
module.
That pass-through was never a deliberate contract. The accessors were
mass-converted from the lustre-local container_of0() by 200d442378
("LU-6142 lustre: convert use of container_of0 in include/") and 46f1fc6c1b
("LU-6142 lustre: convert some container_of to *_safe"), whose commit messages
say the arguments "cannot be determined from local inspection" to be valid –
the old behaviour was simply preserved rather than chosen. The tree has never
been consistent about it either: the lov, lovsub and lod_obj()
accessors have always used plain container_of() for the identical downcast,
and oap2osc() / oap2osc_page() are the same cast written both ways.
The cost is not only the noise. Code has since been added purely to placate
the pass-through:
- the IS_ERR() tests in the llog code from f74ef5903a ("LU-14291 llog: test if dt_device not an ERR_PTR")
- the IS_ERR_OR_NULL() in lod_comp_prep_create() from c53abca9f9 ("
LU-19852lod: raidset aware stripe allocator") - the safety checks in mdt_lproc.c from 117be08e58 ("
LU-18961mdt: add safety checks in MDT_BOOL_RW_ATTR")
and it hides real defects in the noise: five genuine error-pointer bugs were
found underneath this class during triage and are fixed by the first five
patches of this series.