* [PATCH v2 00/13] Introduce seqnum_ops @ 2020-11-13 17:46 Shuah Khan 2020-11-13 17:46 ` [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops Shuah Khan 0 siblings, 1 reply; 3+ messages in thread From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw) To: corbet, keescook, gregkh, peterz, rafael, lenb, james.morse, tony.luck, bp, minyard, arnd, mchehab, rric, valentina.manea.m, shuah, zohar, dmitry.kasatkin, jmorris, serge Cc: Shuah Khan, linux-doc, linux-kernel, linux-kselftest, linux-acpi, openipmi-developer, linux-edac, linux-usb, linux-integrity, linux-security-module Sequence Number api provides interfaces for unsigned atomic up counters leveraging atomic_t and atomic64_t ops underneath. There are a number of atomic_t usages in the kernel where atomic_t api is used for counting sequence numbers and other statistical counters. Several of these usages, convert atomic_read() and atomic_inc_return() return values to unsigned. Introducing sequence number ops supports these use-cases with a standard core-api. The atomic_t api provides a wide range of atomic operations as a base api to implement atomic counters, bitops, spinlock interfaces. The usages also evolved into being used for resource lifetimes and state management. The refcount_t api was introduced to address resource lifetime problems related to atomic_t wrapping. There is a large overlap between the atomic_t api used for resource lifetimes and just counters, stats, and sequence numbers. It has become difficult to differentiate between the atomic_t usages that should be converted to refcount_t and the ones that can be left alone. Introducing seqnum_ops to wrap the usages that are stats, counters, sequence numbers makes it easier for tools that scan for underflow and overflow on atomic_t usages to detect overflow and underflows to scan just the cases that are prone to errors. In addition, to supporting sequence number use-cases, Sequence Number Ops helps differentiate atomic_t counter usages from atomic_t usages that guard object lifetimes, hence prone to overflow and underflow errors from up counting use-cases. It becomes easier for tools that scan for underflow and overflow on atomic_t usages to detect overflow and underflows to scan just the cases that are prone to errors. Changes since v1: - Removed dec based on Greg KH's comments - Removed read/set/inc based on the discussion with Peter Zijlstra - Interfaces are restricted to init, increment and return new value, and fetch current value. - Interfaces return u32 and u64 - a few reviewers suggested unsigned. After reviewing a few use-cases, I determined this is a good path forward. It adds unsigned atomic support that doesn't exist now, and simplifies code in drivers that currently convert atomic_t return values to unsigned. All the drivers changes included in this series used to convert atomic_t returns to unsigned. Patch v1 thread: https://lore.kernel.org/lkml/cover.1605027593.git.skhan@linuxfoundation.org/ Counters thread: lore.kernel.org/lkml/cover.1602209970.git.skhan@linuxfoundation.org Shuah Khan (13): seqnum_ops: Introduce Sequence Number Ops selftests: lib:test_seqnum_ops: add new test for seqnum_ops drivers/acpi: convert seqno seqnum_ops drivers/acpi/apei: convert seqno to seqnum_ops drivers/base/test/test_async_driver_probe: convert to use seqnum_ops drivers/char/ipmi: convert stats to use seqnum_ops drivers/edac: convert pci counters to seqnum_ops drivers/oprofile: convert stats to use seqnum_ops drivers/staging/rtl8723bs: convert stats to use seqnum_ops usb: usbip/vhci: convert seqno to seqnum_ops drivers/staging/rtl8188eu: convert stats to use seqnum_ops drivers/staging/unisys/visorhba: convert stats to use seqnum_ops security/integrity/ima: converts stats to seqnum_ops Documentation/core-api/atomic_ops.rst | 4 + Documentation/core-api/index.rst | 1 + Documentation/core-api/seqnum_ops.rst | 89 +++++++++++++ MAINTAINERS | 8 ++ drivers/acpi/acpi_extlog.c | 8 +- drivers/acpi/apei/ghes.c | 8 +- drivers/base/test/test_async_driver_probe.c | 28 +++-- drivers/char/ipmi/ipmi_msghandler.c | 9 +- drivers/char/ipmi/ipmi_si_intf.c | 9 +- drivers/char/ipmi/ipmi_ssif.c | 9 +- drivers/edac/edac_pci.h | 5 +- drivers/edac/edac_pci_sysfs.c | 30 ++--- drivers/oprofile/buffer_sync.c | 9 +- drivers/oprofile/event_buffer.c | 3 +- drivers/oprofile/oprof.c | 3 +- drivers/oprofile/oprofile_stats.c | 11 +- drivers/oprofile/oprofile_stats.h | 11 +- drivers/oprofile/oprofilefs.c | 3 +- drivers/staging/rtl8188eu/core/rtw_mlme_ext.c | 23 +++- .../staging/rtl8188eu/include/rtw_mlme_ext.h | 3 +- drivers/staging/rtl8723bs/core/rtw_cmd.c | 3 +- drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 33 +++-- drivers/staging/rtl8723bs/include/rtw_cmd.h | 3 +- .../staging/rtl8723bs/include/rtw_mlme_ext.h | 3 +- .../staging/unisys/visorhba/visorhba_main.c | 21 ++-- drivers/usb/usbip/vhci.h | 3 +- drivers/usb/usbip/vhci_hcd.c | 7 +- drivers/usb/usbip/vhci_rx.c | 5 +- include/linux/oprofile.h | 3 +- include/linux/seqnum_ops.h | 118 +++++++++++++++++ lib/Kconfig | 9 ++ lib/Makefile | 1 + lib/test_seqnum_ops.c | 119 ++++++++++++++++++ security/integrity/ima/ima.h | 5 +- security/integrity/ima/ima_api.c | 3 +- security/integrity/ima/ima_fs.c | 5 +- security/integrity/ima/ima_queue.c | 7 +- tools/testing/selftests/lib/Makefile | 1 + tools/testing/selftests/lib/config | 1 + .../testing/selftests/lib/test_seqnum_ops.sh | 10 ++ 40 files changed, 524 insertions(+), 110 deletions(-) create mode 100644 Documentation/core-api/seqnum_ops.rst create mode 100644 include/linux/seqnum_ops.h create mode 100644 lib/test_seqnum_ops.c create mode 100755 tools/testing/selftests/lib/test_seqnum_ops.sh -- 2.27.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops 2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan @ 2020-11-13 17:46 ` Shuah Khan 2020-11-14 16:11 ` kernel test robot 0 siblings, 1 reply; 3+ messages in thread From: Shuah Khan @ 2020-11-13 17:46 UTC (permalink / raw) To: zohar, dmitry.kasatkin, jmorris, serge, gregkh, keescook, peterz Cc: Shuah Khan, linux-security-module, linux-integrity, linux-kernel Sequence Number api provides interfaces for unsigned atomic up counters leveraging atomic_t and atomic64_t ops underneath. Convert it to use seqnum_ops. atomic_t variables used for ima_htable.violations and number of stored measurements and ios_threshold are atomic counters. Convert them to seqnum_ops. Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> --- security/integrity/ima/ima.h | 5 +++-- security/integrity/ima/ima_api.c | 3 ++- security/integrity/ima/ima_fs.c | 5 +++-- security/integrity/ima/ima_queue.c | 7 ++++--- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 6ebefec616e4..55fe1d14c67a 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -21,6 +21,7 @@ #include <linux/tpm.h> #include <linux/audit.h> #include <crypto/hash_info.h> +#include <linux/seqnum_ops.h> #include "../integrity.h" @@ -174,8 +175,8 @@ int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event, extern spinlock_t ima_queue_lock; struct ima_h_table { - atomic_long_t len; /* number of stored measurements in the list */ - atomic_long_t violations; + struct seqnum64 len; /* number of stored measurements in the list */ + struct seqnum64 violations; struct hlist_head queue[IMA_MEASURE_HTABLE_SIZE]; }; extern struct ima_h_table ima_htable; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 4f39fb93f278..c6c442b93ce3 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -14,6 +14,7 @@ #include <linux/xattr.h> #include <linux/evm.h> #include <linux/iversion.h> +#include <linux/seqnum_ops.h> #include "ima.h" @@ -144,7 +145,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, int result; /* can overflow, only indicator */ - atomic_long_inc(&ima_htable.violations); + seqnum64_inc_return(&ima_htable.violations); result = ima_alloc_init_template(&event_data, &entry, NULL); if (result < 0) { diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index ea8ff8a07b36..83a0d33e6f70 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -21,6 +21,7 @@ #include <linux/rcupdate.h> #include <linux/parser.h> #include <linux/vmalloc.h> +#include <linux/seqnum_ops.h> #include "ima.h" @@ -39,12 +40,12 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup); static int valid_policy = 1; static ssize_t ima_show_htable_value(char __user *buf, size_t count, - loff_t *ppos, atomic_long_t *val) + loff_t *ppos, struct seqnum64 *val) { char tmpbuf[32]; /* greater than largest 'long' string value */ ssize_t len; - len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", atomic_long_read(val)); + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val)); return simple_read_from_buffer(buf, count, ppos, tmpbuf, len); } diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index c096ef8945c7..38c31bc62358 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -17,6 +17,7 @@ #include <linux/rculist.h> #include <linux/slab.h> +#include <linux/seqnum_ops.h> #include "ima.h" #define AUDIT_CAUSE_LEN_MAX 32 @@ -33,8 +34,8 @@ static unsigned long binary_runtime_size = ULONG_MAX; /* key: inode (before secure-hashing a file) */ struct ima_h_table ima_htable = { - .len = ATOMIC_LONG_INIT(0), - .violations = ATOMIC_LONG_INIT(0), + .len = SEQNUM_INIT(0), + .violations = SEQNUM_INIT(0), .queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT }; @@ -106,7 +107,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry, INIT_LIST_HEAD(&qe->later); list_add_tail_rcu(&qe->later, &ima_measurements); - atomic_long_inc(&ima_htable.len); + seqnum64_inc_return(&ima_htable.len); if (update_htable) { key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest); hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]); -- 2.27.0 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops 2020-11-13 17:46 ` [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops Shuah Khan @ 2020-11-14 16:11 ` kernel test robot 0 siblings, 0 replies; 3+ messages in thread From: kernel test robot @ 2020-11-14 16:11 UTC (permalink / raw) To: Shuah Khan, zohar, dmitry.kasatkin, jmorris, serge, gregkh, keescook, peterz Cc: kbuild-all, Shuah Khan, linux-security-module, linux-integrity [-- Attachment #1: Type: text/plain, Size: 7214 bytes --] Hi Shuah, I love your patch! Yet something to improve: [auto build test ERROR on staging/staging-testing] [also build test ERROR on integrity/next-integrity char-misc/char-misc-testing usb/usb-testing linus/master v5.10-rc3 next-20201113] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Shuah-Khan/Introduce-seqnum_ops/20201114-014959 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git f4acd33c446b2ba97f1552a4da90050109d01ca7 config: nios2-randconfig-r023-20201114 (attached as .config) compiler: nios2-linux-gcc (GCC) 9.3.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/b86077d3629fe6d16070d95b8331344258dcaed2 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Shuah-Khan/Introduce-seqnum_ops/20201114-014959 git checkout b86077d3629fe6d16070d95b8331344258dcaed2 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=nios2 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All error/warnings (new ones prefixed by >>): In file included from security/integrity/ima/ima_fs.c:26: security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type 178 | struct seqnum64 len; /* number of stored measurements in the list */ | ^~~ security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type 179 | struct seqnum64 violations; | ^~~~~~~~~~ security/integrity/ima/ima_fs.c: In function 'ima_show_htable_value': >> security/integrity/ima/ima_fs.c:48:52: error: implicit declaration of function 'seqnum64_fetch'; did you mean 'seqnum32_fetch'? [-Werror=implicit-function-declaration] 48 | len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val)); | ^~~~~~~~~~~~~~ | seqnum32_fetch >> security/integrity/ima/ima_fs.c:48:46: warning: format '%llu' expects argument of type 'long long unsigned int', but argument 4 has type 'int' [-Wformat=] 48 | len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val)); | ~~~^ ~~~~~~~~~~~~~~~~~~~ | | | | | int | long long unsigned int | %u security/integrity/ima/ima_fs.c: In function 'ima_show_htable_violations': security/integrity/ima/ima_fs.c:57:1: error: control reaches end of non-void function [-Werror=return-type] 57 | } | ^ security/integrity/ima/ima_fs.c: In function 'ima_show_measurements_count': security/integrity/ima/ima_fs.c:70:1: error: control reaches end of non-void function [-Werror=return-type] 70 | } | ^ cc1: some warnings being treated as errors -- In file included from security/integrity/ima/ima_queue.c:21: security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type 178 | struct seqnum64 len; /* number of stored measurements in the list */ | ^~~ security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type 179 | struct seqnum64 violations; | ^~~~~~~~~~ In file included from security/integrity/ima/ima_queue.c:20: include/linux/seqnum_ops.h:40:27: error: field name not in record or union initializer 40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) } | ^ security/integrity/ima/ima_queue.c:37:9: note: in expansion of macro 'SEQNUM_INIT' 37 | .len = SEQNUM_INIT(0), | ^~~~~~~~~~~ include/linux/seqnum_ops.h:40:27: note: (near initialization for 'ima_htable.len') 40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) } | ^ security/integrity/ima/ima_queue.c:37:9: note: in expansion of macro 'SEQNUM_INIT' 37 | .len = SEQNUM_INIT(0), | ^~~~~~~~~~~ include/linux/seqnum_ops.h:40:27: error: field name not in record or union initializer 40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) } | ^ security/integrity/ima/ima_queue.c:38:16: note: in expansion of macro 'SEQNUM_INIT' 38 | .violations = SEQNUM_INIT(0), | ^~~~~~~~~~~ include/linux/seqnum_ops.h:40:27: note: (near initialization for 'ima_htable.violations') 40 | #define SEQNUM_INIT(i) { .seqnum = ATOMIC_INIT(i) } | ^ security/integrity/ima/ima_queue.c:38:16: note: in expansion of macro 'SEQNUM_INIT' 38 | .violations = SEQNUM_INIT(0), | ^~~~~~~~~~~ security/integrity/ima/ima_queue.c: In function 'ima_add_digest_entry': >> security/integrity/ima/ima_queue.c:110:2: error: implicit declaration of function 'seqnum64_inc_return'; did you mean 'seqnum32_inc_return'? [-Werror=implicit-function-declaration] 110 | seqnum64_inc_return(&ima_htable.len); | ^~~~~~~~~~~~~~~~~~~ | seqnum32_inc_return cc1: some warnings being treated as errors -- In file included from security/integrity/ima/ima_api.c:19: security/integrity/ima/ima.h:178:18: error: field 'len' has incomplete type 178 | struct seqnum64 len; /* number of stored measurements in the list */ | ^~~ security/integrity/ima/ima.h:179:18: error: field 'violations' has incomplete type 179 | struct seqnum64 violations; | ^~~~~~~~~~ security/integrity/ima/ima_api.c: In function 'ima_add_violation': >> security/integrity/ima/ima_api.c:148:2: error: implicit declaration of function 'seqnum64_inc_return'; did you mean 'seqnum32_inc_return'? [-Werror=implicit-function-declaration] 148 | seqnum64_inc_return(&ima_htable.violations); | ^~~~~~~~~~~~~~~~~~~ | seqnum32_inc_return cc1: some warnings being treated as errors vim +48 security/integrity/ima/ima_fs.c 41 42 static ssize_t ima_show_htable_value(char __user *buf, size_t count, 43 loff_t *ppos, struct seqnum64 *val) 44 { 45 char tmpbuf[32]; /* greater than largest 'long' string value */ 46 ssize_t len; 47 > 48 len = scnprintf(tmpbuf, sizeof(tmpbuf), "%llu\n", seqnum64_fetch(val)); 49 return simple_read_from_buffer(buf, count, ppos, tmpbuf, len); 50 } 51 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 24371 bytes --] ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-11-14 16:12 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-11-13 17:46 [PATCH v2 00/13] Introduce seqnum_ops Shuah Khan 2020-11-13 17:46 ` [PATCH v2 13/13] security/integrity/ima: converts stats to seqnum_ops Shuah Khan 2020-11-14 16:11 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).