lkml.org 
[lkml]   [2022]   [Jul]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 net 10/12] cipso: Fix data-races around sysctl.
On Wed, Jul 6, 2022 at 7:43 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
>
> While reading cipso sysctl variables, they can be changed concurrently.
> So, we need to add READ_ONCE() to avoid data-races.
>
> Fixes: 446fda4f2682 ("[NetLabel]: CIPSOv4 engine")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> CC: Paul Moore <paul@paul-moore.com>

Thanks for the patch, this looks good to me. However, in the future
you should probably drop the extra "---" separator (just leave the one
before the diffstat below) and move my "Cc:" up above "Fixes:".

Acked-by: Paul Moore <paul@paul-moore.com>

> ---
> Documentation/networking/ip-sysctl.rst | 2 +-
> net/ipv4/cipso_ipv4.c | 12 +++++++-----
> 2 files changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/networking/ip-sysctl.rst b/Documentation/networking/ip-sysctl.rst
> index 9f41961d11d5..0e58001f8580 100644
> --- a/Documentation/networking/ip-sysctl.rst
> +++ b/Documentation/networking/ip-sysctl.rst
> @@ -1085,7 +1085,7 @@ cipso_cache_enable - BOOLEAN
> cipso_cache_bucket_size - INTEGER
> The CIPSO label cache consists of a fixed size hash table with each
> hash bucket containing a number of cache entries. This variable limits
> - the number of entries in each hash bucket; the larger the value the
> + the number of entries in each hash bucket; the larger the value is, the
> more CIPSO label mappings that can be cached. When the number of
> entries in a given hash bucket reaches this limit adding new entries
> causes the oldest entry in the bucket to be removed to make room.
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 62d5f99760aa..6cd3b6c559f0 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -239,7 +239,7 @@ static int cipso_v4_cache_check(const unsigned char *key,
> struct cipso_v4_map_cache_entry *prev_entry = NULL;
> u32 hash;
>
> - if (!cipso_v4_cache_enabled)
> + if (!READ_ONCE(cipso_v4_cache_enabled))
> return -ENOENT;
>
> hash = cipso_v4_map_cache_hash(key, key_len);
> @@ -296,13 +296,14 @@ static int cipso_v4_cache_check(const unsigned char *key,
> int cipso_v4_cache_add(const unsigned char *cipso_ptr,
> const struct netlbl_lsm_secattr *secattr)
> {
> + int bkt_size = READ_ONCE(cipso_v4_cache_bucketsize);
> int ret_val = -EPERM;
> u32 bkt;
> struct cipso_v4_map_cache_entry *entry = NULL;
> struct cipso_v4_map_cache_entry *old_entry = NULL;
> u32 cipso_ptr_len;
>
> - if (!cipso_v4_cache_enabled || cipso_v4_cache_bucketsize <= 0)
> + if (!READ_ONCE(cipso_v4_cache_enabled) || bkt_size <= 0)
> return 0;
>
> cipso_ptr_len = cipso_ptr[1];
> @@ -322,7 +323,7 @@ int cipso_v4_cache_add(const unsigned char *cipso_ptr,
>
> bkt = entry->hash & (CIPSO_V4_CACHE_BUCKETS - 1);
> spin_lock_bh(&cipso_v4_cache[bkt].lock);
> - if (cipso_v4_cache[bkt].size < cipso_v4_cache_bucketsize) {
> + if (cipso_v4_cache[bkt].size < bkt_size) {
> list_add(&entry->list, &cipso_v4_cache[bkt].list);
> cipso_v4_cache[bkt].size += 1;
> } else {
> @@ -1199,7 +1200,8 @@ static int cipso_v4_gentag_rbm(const struct cipso_v4_doi *doi_def,
> /* This will send packets using the "optimized" format when
> * possible as specified in section 3.4.2.6 of the
> * CIPSO draft. */
> - if (cipso_v4_rbm_optfmt && ret_val > 0 && ret_val <= 10)
> + if (READ_ONCE(cipso_v4_rbm_optfmt) && ret_val > 0 &&
> + ret_val <= 10)
> tag_len = 14;
> else
> tag_len = 4 + ret_val;
> @@ -1603,7 +1605,7 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
> * all the CIPSO validations here but it doesn't
> * really specify _exactly_ what we need to validate
> * ... so, just make it a sysctl tunable. */
> - if (cipso_v4_rbm_strictvalid) {
> + if (READ_ONCE(cipso_v4_rbm_strictvalid)) {
> if (cipso_v4_map_lvl_valid(doi_def,
> tag[3]) < 0) {
> err_offset = opt_iter + 3;
> --
> 2.30.2

--
paul-moore.com

\
 
 \ /
  Last update: 2022-07-07 21:17    [W:0.108 / U:0.336 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site