| Message ID | 20241205063023.744685-1-15074444048@163.com (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | [v2] kernel/audit: reorder the member of audit_context | expand |
On Dec 5, 2024 15074444048@163.com wrote: > > Adjust the positions of target_ref and target_comm to > reduce their direct proximity. This can prevent the memset > operation on target_ref from potentially affecting target_comm I don't really see the above as very convinving, sure by moving the target_ref field after target_comm it is less likely that a too-long memset() on target_ref will impact target_comm, but ... * lsm_prop is a fixed sized struct so memset()'ing it should be easy to verify. * if a memset(target_ref) does go long, it still jeapordizes other parts of the audit_context struct (see the audit_tree_refs pointers). If we need to workaround some shortcomings of the static analysis tooling (which compiler/tool are you using?) that's okay, and the paragraph below is fine, but I don't believe this change really offers much in the way of runtime protection (the first paragraph). I'm CC'ing the linux-hardening folks as they may have some additional insight on how to best work around static analysis limitations. > The compiler's static analysis tool considers that the memset > operation might affect the adjacent struct member target_comm, > potentially causing subsequent writes to target_comm to exceed > its boundaries. Even if an overflow does not actually occur, > the compiler may issue a warning because it cannot fully > determine the safety of the operation. > > Verification: > - "pahole" shows no size nor member offset changes to struct audit_context. > - "objdump -d" shows no object code changes. > > Signed-off-by: lihaojie <lihaojie@kylinos.cn> > --- > kernel/audit.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/audit.h b/kernel/audit.h > index 0211cb307d30..77c140228a4a 100644 > --- a/kernel/audit.h > +++ b/kernel/audit.h > @@ -144,8 +144,8 @@ struct audit_context { > kuid_t target_auid; > kuid_t target_uid; > unsigned int target_sessionid; > - struct lsm_prop target_ref; > char target_comm[TASK_COMM_LEN]; > + struct lsm_prop target_ref; > > struct audit_tree_refs *trees, *first_trees; > struct list_head killed_trees; > -- > 2.25.1 -- paul-moore.com
diff --git a/kernel/audit.h b/kernel/audit.h index 0211cb307d30..77c140228a4a 100644 --- a/kernel/audit.h +++ b/kernel/audit.h @@ -144,8 +144,8 @@ struct audit_context { kuid_t target_auid; kuid_t target_uid; unsigned int target_sessionid; - struct lsm_prop target_ref; char target_comm[TASK_COMM_LEN]; + struct lsm_prop target_ref; struct audit_tree_refs *trees, *first_trees; struct list_head killed_trees;