Re: [PATCH -next 0/9] lockdown: fix the securityfs interface and add tests

[nicolas . bouchinet]

Hi Cai,

Thanks for your contribution.
> The lockdown LSM exposes /sys/kernel/security/lockdown, but the file is
> not documented and has no selftest.  While looking at the interface we
> also found a configuration bug and several accumulated cleanups.
>
> The series is organized as follows:
>
>   - patches 1-2 are small cleanups in lockdown_write(): stop scanning
>     after the matching level and drop an update to a length variable
>     that is not used afterwards.
>   - patch 3 replaces sprintf() with scnprintf() so that the status
>     buffer is bounded explicitly if more levels are added later.
>   - patch 4 makes the LSM select SECURITYFS.  Without it, a kernel
>     built with CONFIG_SECURITYFS=n logs "failed LSM lockdown core
>     initcall with errno -19" and does not create the file advertised
>     by the Kconfig help text.
>   - patch 5 removes LSM_AUDIT_DATA_LOCKDOWN and its formatting code,
>     which became dead when the SELinux lockdown implementation was
>     removed by commit f5d0e5e9d72d.
>   - patches 6-7 drop an unused include, simplify the securityfs
>     registration, and move lockdown_reasons[] next to its only user.
>   - patch 8 documents the ABI and adds the new file to the LOCKDOWN
>     SECURITY MODULE entry in MAINTAINERS.
>   - patch 9 adds a kselftest covering the read format, invalid levels,
>     and the rejection of the current or a lower level.
>
> The only functional change is patch 4; the rest are cleanups,
> documentation, or test coverage.  The selftest never escalates
> lockdown on the machine where it runs.
>
> Cai Xinchen (8):
>   lockdown: Add break in lockdown_write
>   lockdown: bound the status string with scnprintf
>   lockdown: select SECURITYFS
>   lsm: remove unused lockdown audit data
>   lockdown: drop an unused include and simplify secfs init
>   lockdown: move lockdown_reasons next to its only user
>   Documentation/ABI: document the lockdown securityfs file
>   selftests/lsm: add lockdown securityfs test
>
> Kalevi Kolttonen (1):
>   lockdown: remove useless decrement operation
>
>  Documentation/ABI/testing/securityfs-lockdown |  41 ++++
>  MAINTAINERS                                   |   1 +
>  include/linux/lsm_audit.h                     |   2 -
>  include/linux/security.h                      |   2 -
>  security/lockdown/Kconfig                     |   1 +
>  security/lockdown/lockdown.c                  |  63 +++++--
>  security/lsm_audit.c                          |   4 -
>  security/security.c                           |  40 ----
>  tools/testing/selftests/lsm/Makefile          |   3 +-
>  tools/testing/selftests/lsm/common.c          |  35 +++-
>  tools/testing/selftests/lsm/common.h          |   2 +
>  tools/testing/selftests/lsm/lockdown_test.c   | 176 ++++++++++++++++++
>  12 files changed, 300 insertions(+), 70 deletions(-)
>  create mode 100644 Documentation/ABI/testing/securityfs-lockdown
>  create mode 100644 tools/testing/selftests/lsm/lockdown_test.c

Those looks good to me, I've added comments in patch 9.
I'm not at work this week so I'll tests your series next monday. Sorry
for the inconvenience.

Best regards,

Nicolas