* [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
2021-01-10 17:33 [RFC PATCH v2 0/8] Count rlimits in each user namespace Alexey Gladkov
@ 2021-01-10 17:33 ` Alexey Gladkov
0 siblings, 0 replies; 7+ messages in thread
From: Alexey Gladkov @ 2021-01-10 17:33 UTC (permalink / raw)
To: LKML, Linux Containers, Kernel Hardening
Cc: Linus Torvalds, Alexey Gladkov, Eric W . Biederman,
Christian Brauner, Kees Cook
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
include/linux/user_namespace.h | 2 +-
kernel/ucount.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 64cf8ebdc4ec..84fefa9247c4 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -92,7 +92,7 @@ struct ucounts {
struct hlist_node node;
struct user_namespace *ns;
kuid_t uid;
- int count;
+ atomic_t count;
atomic_t ucount[UCOUNT_COUNTS];
};
diff --git a/kernel/ucount.c b/kernel/ucount.c
index 11b1596e2542..0f2c7c11df19 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -141,7 +141,8 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
new->ns = ns;
new->uid = uid;
- new->count = 0;
+
+ atomic_set(&new->count, 0);
spin_lock_irq(&ucounts_lock);
ucounts = find_ucounts(ns, uid, hashent);
@@ -152,10 +153,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
ucounts = new;
}
}
- if (ucounts->count == INT_MAX)
+ if (atomic_read(&ucounts->count) == INT_MAX)
ucounts = NULL;
else
- ucounts->count += 1;
+ atomic_inc(&ucounts->count);
spin_unlock_irq(&ucounts_lock);
return ucounts;
}
@@ -165,8 +166,7 @@ static void put_ucounts(struct ucounts *ucounts)
unsigned long flags;
spin_lock_irqsave(&ucounts_lock, flags);
- ucounts->count -= 1;
- if (!ucounts->count)
+ if (atomic_dec_and_test(&ucounts->count))
hlist_del_init(&ucounts->node);
else
ucounts = NULL;
--
2.29.2
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
@ 2021-01-10 17:33 ` Alexey Gladkov
0 siblings, 0 replies; 7+ messages in thread
From: Alexey Gladkov @ 2021-01-10 17:33 UTC (permalink / raw)
To: LKML, Linux Containers, Kernel Hardening
Cc: Alexey Gladkov, Eric W . Biederman, Kees Cook, Christian Brauner,
Linus Torvalds
Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
---
include/linux/user_namespace.h | 2 +-
kernel/ucount.c | 10 +++++-----
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
index 64cf8ebdc4ec..84fefa9247c4 100644
--- a/include/linux/user_namespace.h
+++ b/include/linux/user_namespace.h
@@ -92,7 +92,7 @@ struct ucounts {
struct hlist_node node;
struct user_namespace *ns;
kuid_t uid;
- int count;
+ atomic_t count;
atomic_t ucount[UCOUNT_COUNTS];
};
diff --git a/kernel/ucount.c b/kernel/ucount.c
index 11b1596e2542..0f2c7c11df19 100644
--- a/kernel/ucount.c
+++ b/kernel/ucount.c
@@ -141,7 +141,8 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
new->ns = ns;
new->uid = uid;
- new->count = 0;
+
+ atomic_set(&new->count, 0);
spin_lock_irq(&ucounts_lock);
ucounts = find_ucounts(ns, uid, hashent);
@@ -152,10 +153,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
ucounts = new;
}
}
- if (ucounts->count == INT_MAX)
+ if (atomic_read(&ucounts->count) == INT_MAX)
ucounts = NULL;
else
- ucounts->count += 1;
+ atomic_inc(&ucounts->count);
spin_unlock_irq(&ucounts_lock);
return ucounts;
}
@@ -165,8 +166,7 @@ static void put_ucounts(struct ucounts *ucounts)
unsigned long flags;
spin_lock_irqsave(&ucounts_lock, flags);
- ucounts->count -= 1;
- if (!ucounts->count)
+ if (atomic_dec_and_test(&ucounts->count))
hlist_del_init(&ucounts->node);
else
ucounts = NULL;
--
2.29.2
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
@ 2021-01-10 22:34 kernel test robot
0 siblings, 0 replies; 7+ messages in thread
From: kernel test robot @ 2021-01-10 22:34 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 2738 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <447547b12bba1894d3f1f79d6408dfc60b219b0c.1610299857.git.gladkov.alexey@gmail.com>
References: <447547b12bba1894d3f1f79d6408dfc60b219b0c.1610299857.git.gladkov.alexey@gmail.com>
TO: Alexey Gladkov <gladkov.alexey@gmail.com>
Hi Alexey,
[FYI, it's a private test report for your RFC patch.]
[auto build test WARNING on linus/master]
[also build test WARNING on hnaz-linux-mm/master v5.11-rc2 next-20210108]
[cannot apply to linux/master kees/for-next/pstore]
[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/Alexey-Gladkov/Count-rlimits-in-each-user-namespace/20210111-014938
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2ff90100ace886895e4fbb2850b8d5e49d931ed6
:::::: branch date: 5 hours ago
:::::: commit date: 5 hours ago
config: mips-randconfig-c003-20210111 (attached as .config)
compiler: mips64-linux-gcc (GCC) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Julia Lawall <julia.lawall@lip6.fr>
"coccinelle warnings: (new ones prefixed by >>)"
>> kernel/ucount.c:169:5-24: atomic_dec_and_test variation before object free at line 170.
kernel/ucount.c:169:5-24: atomic_dec_and_test variation before object free at line 175.
vim +169 kernel/ucount.c
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 163
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 164 static void put_ucounts(struct ucounts *ucounts)
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 165 {
880a38547ff08715 Nikolay Borisov 2017-01-20 166 unsigned long flags;
880a38547ff08715 Nikolay Borisov 2017-01-20 167
880a38547ff08715 Nikolay Borisov 2017-01-20 168 spin_lock_irqsave(&ucounts_lock, flags);
e58c759c8718b56a Alexey Gladkov 2021-01-10 @169 if (atomic_dec_and_test(&ucounts->count))
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 @170 hlist_del_init(&ucounts->node);
040757f738e13caa Eric W. Biederman 2017-03-05 171 else
040757f738e13caa Eric W. Biederman 2017-03-05 172 ucounts = NULL;
880a38547ff08715 Nikolay Borisov 2017-01-20 173 spin_unlock_irqrestore(&ucounts_lock, flags);
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 174
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 175 kfree(ucounts);
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 176 }
f6b2db1a3e8d141d Eric W. Biederman 2016-08-08 177
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 38685 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
2021-01-10 17:33 ` Alexey Gladkov
@ 2021-01-13 16:31 ` Eric W. Biederman
-1 siblings, 0 replies; 7+ messages in thread
From: Eric W. Biederman @ 2021-01-13 16:31 UTC (permalink / raw)
To: Alexey Gladkov
Cc: Kees Cook, Kernel Hardening, Linus Torvalds, Linux Containers,
LKML, Alexey Gladkov, Christian Brauner
Alexey Gladkov <gladkov.alexey@gmail.com> writes:
We might want to use refcount_t instead of atomic_t. Not a big deal
either way.
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
> include/linux/user_namespace.h | 2 +-
> kernel/ucount.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 64cf8ebdc4ec..84fefa9247c4 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -92,7 +92,7 @@ struct ucounts {
> struct hlist_node node;
> struct user_namespace *ns;
> kuid_t uid;
> - int count;
> + atomic_t count;
> atomic_t ucount[UCOUNT_COUNTS];
> };
>
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 11b1596e2542..0f2c7c11df19 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -141,7 +141,8 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
>
> new->ns = ns;
> new->uid = uid;
> - new->count = 0;
> +
> + atomic_set(&new->count, 0);
>
> spin_lock_irq(&ucounts_lock);
> ucounts = find_ucounts(ns, uid, hashent);
> @@ -152,10 +153,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
> ucounts = new;
> }
> }
> - if (ucounts->count == INT_MAX)
> + if (atomic_read(&ucounts->count) == INT_MAX)
> ucounts = NULL;
> else
> - ucounts->count += 1;
> + atomic_inc(&ucounts->count);
> spin_unlock_irq(&ucounts_lock);
> return ucounts;
> }
> @@ -165,8 +166,7 @@ static void put_ucounts(struct ucounts *ucounts)
> unsigned long flags;
>
> spin_lock_irqsave(&ucounts_lock, flags);
> - ucounts->count -= 1;
> - if (!ucounts->count)
> + if (atomic_dec_and_test(&ucounts->count))
> hlist_del_init(&ucounts->node);
> else
> ucounts = NULL;
This can become:
static void put_ucounts(struct ucounts *ucounts)
{
unsigned long flags;
if (atomic_dec_and_lock_irqsave(&ucounts->count, &ucounts_lock, flags)) {
hlist_del_init(&ucounts->node);
spin_unlock_irqrestore(&ucounts_lock);
kfree(ucounts);
}
}
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
@ 2021-01-13 16:31 ` Eric W. Biederman
0 siblings, 0 replies; 7+ messages in thread
From: Eric W. Biederman @ 2021-01-13 16:31 UTC (permalink / raw)
To: Alexey Gladkov
Cc: LKML, Linux Containers, Kernel Hardening, Alexey Gladkov,
Kees Cook, Christian Brauner, Linus Torvalds
Alexey Gladkov <gladkov.alexey@gmail.com> writes:
We might want to use refcount_t instead of atomic_t. Not a big deal
either way.
> Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> ---
> include/linux/user_namespace.h | 2 +-
> kernel/ucount.c | 10 +++++-----
> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> index 64cf8ebdc4ec..84fefa9247c4 100644
> --- a/include/linux/user_namespace.h
> +++ b/include/linux/user_namespace.h
> @@ -92,7 +92,7 @@ struct ucounts {
> struct hlist_node node;
> struct user_namespace *ns;
> kuid_t uid;
> - int count;
> + atomic_t count;
> atomic_t ucount[UCOUNT_COUNTS];
> };
>
> diff --git a/kernel/ucount.c b/kernel/ucount.c
> index 11b1596e2542..0f2c7c11df19 100644
> --- a/kernel/ucount.c
> +++ b/kernel/ucount.c
> @@ -141,7 +141,8 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
>
> new->ns = ns;
> new->uid = uid;
> - new->count = 0;
> +
> + atomic_set(&new->count, 0);
>
> spin_lock_irq(&ucounts_lock);
> ucounts = find_ucounts(ns, uid, hashent);
> @@ -152,10 +153,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
> ucounts = new;
> }
> }
> - if (ucounts->count == INT_MAX)
> + if (atomic_read(&ucounts->count) == INT_MAX)
> ucounts = NULL;
> else
> - ucounts->count += 1;
> + atomic_inc(&ucounts->count);
> spin_unlock_irq(&ucounts_lock);
> return ucounts;
> }
> @@ -165,8 +166,7 @@ static void put_ucounts(struct ucounts *ucounts)
> unsigned long flags;
>
> spin_lock_irqsave(&ucounts_lock, flags);
> - ucounts->count -= 1;
> - if (!ucounts->count)
> + if (atomic_dec_and_test(&ucounts->count))
> hlist_del_init(&ucounts->node);
> else
> ucounts = NULL;
This can become:
static void put_ucounts(struct ucounts *ucounts)
{
unsigned long flags;
if (atomic_dec_and_lock_irqsave(&ucounts->count, &ucounts_lock, flags)) {
hlist_del_init(&ucounts->node);
spin_unlock_irqrestore(&ucounts_lock);
kfree(ucounts);
}
}
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
2021-01-13 16:31 ` Eric W. Biederman
@ 2021-01-13 18:01 ` Kees Cook
-1 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2021-01-13 18:01 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Kernel Hardening, Linux Containers, LKML, Linus Torvalds,
Alexey Gladkov, Christian Brauner
On Wed, Jan 13, 2021 at 10:31:40AM -0600, Eric W. Biederman wrote:
> Alexey Gladkov <gladkov.alexey@gmail.com> writes:
>
> We might want to use refcount_t instead of atomic_t. Not a big deal
> either way.
Yes, please use refcount_t, and don't use _read() since that introduces
races.
-Kees
>
> > Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> > ---
> > include/linux/user_namespace.h | 2 +-
> > kernel/ucount.c | 10 +++++-----
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> > index 64cf8ebdc4ec..84fefa9247c4 100644
> > --- a/include/linux/user_namespace.h
> > +++ b/include/linux/user_namespace.h
> > @@ -92,7 +92,7 @@ struct ucounts {
> > struct hlist_node node;
> > struct user_namespace *ns;
> > kuid_t uid;
> > - int count;
> > + atomic_t count;
> > atomic_t ucount[UCOUNT_COUNTS];
> > };
> >
> > diff --git a/kernel/ucount.c b/kernel/ucount.c
> > index 11b1596e2542..0f2c7c11df19 100644
> > --- a/kernel/ucount.c
> > +++ b/kernel/ucount.c
> > @@ -141,7 +141,8 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
> >
> > new->ns = ns;
> > new->uid = uid;
> > - new->count = 0;
> > +
> > + atomic_set(&new->count, 0);
> >
> > spin_lock_irq(&ucounts_lock);
> > ucounts = find_ucounts(ns, uid, hashent);
> > @@ -152,10 +153,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
> > ucounts = new;
> > }
> > }
> > - if (ucounts->count == INT_MAX)
> > + if (atomic_read(&ucounts->count) == INT_MAX)
> > ucounts = NULL;
> > else
> > - ucounts->count += 1;
> > + atomic_inc(&ucounts->count);
> > spin_unlock_irq(&ucounts_lock);
> > return ucounts;
> > }
> > @@ -165,8 +166,7 @@ static void put_ucounts(struct ucounts *ucounts)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&ucounts_lock, flags);
> > - ucounts->count -= 1;
> > - if (!ucounts->count)
> > + if (atomic_dec_and_test(&ucounts->count))
> > hlist_del_init(&ucounts->node);
> > else
> > ucounts = NULL;
>
>
> This can become:
> static void put_ucounts(struct ucounts *ucounts)
> {
> unsigned long flags;
>
> if (atomic_dec_and_lock_irqsave(&ucounts->count, &ucounts_lock, flags)) {
> hlist_del_init(&ucounts->node);
> spin_unlock_irqrestore(&ucounts_lock);
> kfree(ucounts);
> }
> }
>
--
Kees Cook
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting
@ 2021-01-13 18:01 ` Kees Cook
0 siblings, 0 replies; 7+ messages in thread
From: Kees Cook @ 2021-01-13 18:01 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Alexey Gladkov, LKML, Linux Containers, Kernel Hardening,
Alexey Gladkov, Christian Brauner, Linus Torvalds
On Wed, Jan 13, 2021 at 10:31:40AM -0600, Eric W. Biederman wrote:
> Alexey Gladkov <gladkov.alexey@gmail.com> writes:
>
> We might want to use refcount_t instead of atomic_t. Not a big deal
> either way.
Yes, please use refcount_t, and don't use _read() since that introduces
races.
-Kees
>
> > Signed-off-by: Alexey Gladkov <gladkov.alexey@gmail.com>
> > ---
> > include/linux/user_namespace.h | 2 +-
> > kernel/ucount.c | 10 +++++-----
> > 2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h
> > index 64cf8ebdc4ec..84fefa9247c4 100644
> > --- a/include/linux/user_namespace.h
> > +++ b/include/linux/user_namespace.h
> > @@ -92,7 +92,7 @@ struct ucounts {
> > struct hlist_node node;
> > struct user_namespace *ns;
> > kuid_t uid;
> > - int count;
> > + atomic_t count;
> > atomic_t ucount[UCOUNT_COUNTS];
> > };
> >
> > diff --git a/kernel/ucount.c b/kernel/ucount.c
> > index 11b1596e2542..0f2c7c11df19 100644
> > --- a/kernel/ucount.c
> > +++ b/kernel/ucount.c
> > @@ -141,7 +141,8 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
> >
> > new->ns = ns;
> > new->uid = uid;
> > - new->count = 0;
> > +
> > + atomic_set(&new->count, 0);
> >
> > spin_lock_irq(&ucounts_lock);
> > ucounts = find_ucounts(ns, uid, hashent);
> > @@ -152,10 +153,10 @@ static struct ucounts *get_ucounts(struct user_namespace *ns, kuid_t uid)
> > ucounts = new;
> > }
> > }
> > - if (ucounts->count == INT_MAX)
> > + if (atomic_read(&ucounts->count) == INT_MAX)
> > ucounts = NULL;
> > else
> > - ucounts->count += 1;
> > + atomic_inc(&ucounts->count);
> > spin_unlock_irq(&ucounts_lock);
> > return ucounts;
> > }
> > @@ -165,8 +166,7 @@ static void put_ucounts(struct ucounts *ucounts)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&ucounts_lock, flags);
> > - ucounts->count -= 1;
> > - if (!ucounts->count)
> > + if (atomic_dec_and_test(&ucounts->count))
> > hlist_del_init(&ucounts->node);
> > else
> > ucounts = NULL;
>
>
> This can become:
> static void put_ucounts(struct ucounts *ucounts)
> {
> unsigned long flags;
>
> if (atomic_dec_and_lock_irqsave(&ucounts->count, &ucounts_lock, flags)) {
> hlist_del_init(&ucounts->node);
> spin_unlock_irqrestore(&ucounts_lock);
> kfree(ucounts);
> }
> }
>
--
Kees Cook
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-13 18:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-10 22:34 [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2021-01-10 17:33 [RFC PATCH v2 0/8] Count rlimits in each user namespace Alexey Gladkov
2021-01-10 17:33 ` [RFC PATCH v2 1/8] Use atomic type for ucounts reference counting Alexey Gladkov
2021-01-10 17:33 ` Alexey Gladkov
2021-01-13 16:31 ` Eric W. Biederman
2021-01-13 16:31 ` Eric W. Biederman
2021-01-13 18:01 ` Kees Cook
2021-01-13 18:01 ` Kees Cook
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.