[PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Matthew Wilcox (Oracle)]

Commit 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")
attempted to fix the problem of secretmem_users wrapping to zero and
allowing suspend once again.  Prevent secretmem_users from wrapping to
zero by forbidding new users if the number of users has wrapped from
positive to negative.  This stops a long way short of reaching the
necessary 4 billion users, so there's no need to be clever with special
anti-wrap types or checking the return value from atomic_inc().

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Cc: Jordy Zomer <jordy@pwning.systems>
Cc: Kees Cook <keescook@chromium.org>,
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Mike Rapoport <rppt@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 mm/secretmem.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/secretmem.c b/mm/secretmem.c
index 030f02ddc7c1..c2dda408bb36 100644
--- a/mm/secretmem.c
+++ b/mm/secretmem.c
@@ -203,6 +203,8 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
 
 	if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))
 		return -EINVAL;
+	if (atomic_read(&secretmem_users) < 0)
+		return -ENFILE;
 
 	fd = get_unused_fd_flags(flags & O_CLOEXEC);
 	if (fd < 0)
-- 
2.33.0

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Kees Cook]

On Mon, Oct 25, 2021 at 07:16:34PM +0100, Matthew Wilcox (Oracle) wrote:
> Commit 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")
> attempted to fix the problem of secretmem_users wrapping to zero and
> allowing suspend once again.  Prevent secretmem_users from wrapping to
> zero by forbidding new users if the number of users has wrapped from
> positive to negative.  This stops a long way short of reaching the
> necessary 4 billion users, so there's no need to be clever with special
> anti-wrap types or checking the return value from atomic_inc().

I still prefer refcount_t here because it provides deterministic
saturation, but the risk right now is so narrow ("don't hibernate"),
I'm not going to fight for it. I think it'd be fine to use it initialized
to 1, and have the removal check for == 0 as a failure state, which would
deterministically cover the underflow case too.

-Kees

> 
> Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
> Cc: Jordy Zomer <jordy@pwning.systems>
> Cc: Kees Cook <keescook@chromium.org>,
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> Cc: Mike Rapoport <rppt@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  mm/secretmem.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/mm/secretmem.c b/mm/secretmem.c
> index 030f02ddc7c1..c2dda408bb36 100644
> --- a/mm/secretmem.c
> +++ b/mm/secretmem.c
> @@ -203,6 +203,8 @@ SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
>  
>  	if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))
>  		return -EINVAL;
> +	if (atomic_read(&secretmem_users) < 0)
> +		return -ENFILE;
>  
>  	fd = get_unused_fd_flags(flags & O_CLOEXEC);
>  	if (fd < 0)
> -- 
> 2.33.0
> 

-- 
Kees Cook

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Matthew Wilcox]

On Mon, Oct 25, 2021 at 12:29:46PM -0700, Kees Cook wrote:
> On Mon, Oct 25, 2021 at 07:16:34PM +0100, Matthew Wilcox (Oracle) wrote:
> > Commit 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")
> > attempted to fix the problem of secretmem_users wrapping to zero and
> > allowing suspend once again.  Prevent secretmem_users from wrapping to
> > zero by forbidding new users if the number of users has wrapped from
> > positive to negative.  This stops a long way short of reaching the
> > necessary 4 billion users, so there's no need to be clever with special
> > anti-wrap types or checking the return value from atomic_inc().
> 
> I still prefer refcount_t here because it provides deterministic
> saturation, but the risk right now is so narrow ("don't hibernate"),
> I'm not going to fight for it. I think it'd be fine to use it initialized
> to 1, and have the removal check for == 0 as a failure state, which would
> deterministically cover the underflow case too.

I still think that's abusing the refcount_t pattern.  refcount_t should
be for ... reference counts.  Not these other things.

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Kees Cook]

On Mon, Oct 25, 2021 at 08:51:40PM +0100, Matthew Wilcox wrote:
> On Mon, Oct 25, 2021 at 12:29:46PM -0700, Kees Cook wrote:
> > On Mon, Oct 25, 2021 at 07:16:34PM +0100, Matthew Wilcox (Oracle) wrote:
> > > Commit 110860541f44 ("mm/secretmem: use refcount_t instead of atomic_t")
> > > attempted to fix the problem of secretmem_users wrapping to zero and
> > > allowing suspend once again.  Prevent secretmem_users from wrapping to
> > > zero by forbidding new users if the number of users has wrapped from
> > > positive to negative.  This stops a long way short of reaching the
> > > necessary 4 billion users, so there's no need to be clever with special
> > > anti-wrap types or checking the return value from atomic_inc().
> > 
> > I still prefer refcount_t here because it provides deterministic
> > saturation, but the risk right now is so narrow ("don't hibernate"),
> > I'm not going to fight for it. I think it'd be fine to use it initialized
> > to 1, and have the removal check for == 0 as a failure state, which would
> > deterministically cover the underflow case too.
> 
> I still think that's abusing the refcount_t pattern.  refcount_t should
> be for ... reference counts.  Not these other things.

Is secretmem different? We're trying to count how many of these we have,
this is a common pattern in, for example, the network code which does
this kind of thing a lot.

It's just that for a "global" counter like here, we're not tied to a
specific object's lifetime, but usually some global state that depends
on having _any_ of the objects alive.

-- 
Kees Cook

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Linus Torvalds]

On Mon, Oct 25, 2021 at 2:04 PM Kees Cook <keescook@chromium.org> wrote:
>
> Is secretmem different? We're trying to count how many of these we have,
> this is a common pattern in, for example, the network code which does
> this kind of thing a lot.

Yes, secretmem is different.

A refcount being zero means that the data it referenced no longer exists.

That's not what the secretmem counter meant at all.

Making it a refcount was simply WRONG. It's not a refcount, it's a
usage count, and the fact that syzbot caught the warning just shows
how wrong it was.

Stop arguing for garbage. It was wrong, just admit it. The semantics
for "refcount" is something else than what that code had. As a result,
it got reverted. I've applied Willy's patch that actually makes sense.

Arguing for garbage in the name of "security" is still garbage. In
fact, it only causes confusion, and as such is likely to result in
problems - including security problems - later.

Because confusion about semantics is bad. And that was what that patch was.

And I want to state - again - how dangerous this "refcounts are always
prefereable to atomics" mental model is. Refcounts are _not_
fundamentally preferable to atomics. They are slower, bigger, and have
completely different semantics.

So if something isn't a refcount, it damn well shouldn't use "refcount_t".

             Linus

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Kees Cook]

On Mon, Oct 25, 2021 at 02:17:58PM -0700, Linus Torvalds wrote:
> On Mon, Oct 25, 2021 at 2:04 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Is secretmem different? We're trying to count how many of these we have,
> > this is a common pattern in, for example, the network code which does
> > this kind of thing a lot.
> 
> Yes, secretmem is different.

Prefix: I'm fine with this being whatever; it's a corner case, so this
reply is mainly about nailing down the rationales for future decisions.

> A refcount being zero means that the data it referenced no longer exists.

I don't disagree with this definition, but I would like to understand how
some other use-cases fit into this. The case of basic allocated object memory
lifetime reference counting we all agree on. What about the case of what
I see that is more like a "shared resource usage count" where the shared
resource doesn't necessarily disappear when we reach "no users"?

i.e. there is some resource, and it starts its life with no one using it
(count = 1). Then we have users declaring their use (count = 2) and later
release (count = 1) of the resource. It's not really ever unallocated when
users == 0 (count == 1), but we might use the usage counter for other
things and don't want to let it go negative or ever increment from zero
(in case zero is used for marking the resource unavailable forever). For
example, protocols knowing if there are any sockets left open, crypto
API usage counts, kernel module usage counts, etc.

I don't see as clear a distinction between secretmem and the above
examples. The question being answered is "how many users do I have?" and
we want to make sure we don't end up with overflow or underflow given the
(unfortunately) common case of reference counting kernel bugs. The fact
that secretmem doesn't have its own allocation to free when it hits 0
seems like just an implementation detail of this particular resource
usage counter.

But, ignoring secretmem for a moment, what about a specific example
from above: the module loader's refcnt atomic_t, which is actually
maintaining an allocation (the module), and uses usage counts of 0 to mean
"I am removing this module", and 1 is "I have no users", 2 is "1 user",
etc. It seems like this should use refcount_t to me?

> Stop arguing for garbage. It was wrong, just admit it. The semantics
> for "refcount" is something else than what that code had. As a result,
> it got reverted. I've applied Willy's patch that actually makes sense.
> 
> Arguing for garbage in the name of "security" is still garbage. In
> fact, it only causes confusion, and as such is likely to result in
> problems - including security problems - later.

Sure, and reasonable people can disagree about what is garbage. :) I see
using a refcount_t here as a not unreasonable way to protect against
potential future security problems if the scope of secretmem ever grows
and more than just hibernation starts to care about this usage counter.

> Because confusion about semantics is bad. And that was what that patch was.
> 
> And I want to state - again - how dangerous this "refcounts are always
> prefereable to atomics" mental model is. Refcounts are _not_
> fundamentally preferable to atomics. They are slower, bigger, and have
> completely different semantics.

I will push back on this; I don't think that's a fair assessment at all.
For storage size, refcount_t is identical to atomic_t (refcount_t _is_
an atomic_t). But perhaps you meant code size? Yes, the refcount_t
helpers are technically larger. But like speed, where refcount_t is also
technically slower, neither size nor speed were so much changed that
we kept around the routines that made it exactly the same speed and
grew the instruction count by 1 (the original x86-specific refcount_t
implementation was just as fast as atomic_t). I don't see speed nor size
alone to be a good reason to say "don't use refcount_t".

But yes, I agree about the different semantics: there are very specific
memory ordering assumptions that tend to be more strict than atomic_t
(which IMO actually makes it more suitable than atomic_t for shared
usage counts).

> So if something isn't a refcount, it damn well shouldn't use "refcount_t".

Again, I don't disagree, but since it looked like a refcount to me, I'd
like to understand what why we don't see this case the same way. Since I
agree that secretmem is currently pretty iffy (nothing actually allocated
to track, only system state), I'll ask a slightly different question:
should the module loader use refcount_t? If not, why?

-- 
Kees Cook

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Linus Torvalds]

On Mon, Oct 25, 2021 at 3:30 PM Kees Cook <keescook@chromium.org> wrote:
>
> > A refcount being zero means that the data it referenced no longer exists.
>
> I don't disagree with this definition, but I would like to understand how
> some other use-cases fit into this.

I certainly hope that there are no other use-cases for 'recount_t',
because that "zero is invalid" is very much part of the semantics.

If we want other semantics, it should be a new type.

>      What about the case of what
> I see that is more like a "shared resource usage count" where the shared
> resource doesn't necessarily disappear when we reach "no users"?

So I think that's really "atomic_t".

And instead of saturating, people should always check such shared
resources for limits.

> i.e. there is some resource, and it starts its life with no one using it
> (count = 1).

You are already going off into the weeds.

That's not a natural thing to do. It's already confusing. Really. Read
that sentence yourself, and read it like an outsider.

"No one is using it, so count == 1" is a nonsensican statement on the
face of it.

You are thinking of a refcount_t trick, not some sane semantics.

Yes, we have played off-by-one games in the kernel before. We've done
it for various subtle reasons.

For example, traditionally, on x86, with atomic counting there are
three special situations: negative, 0 and positive. So if you use the
traditional x86 counting atomics (just add/sub/inc/dec, no xadd) then
there are situations where you can get more information about the
result in %eflags if you don't use zero as the initial value, but -1.

Because then you can do "inc", and if ZF is set, you know you were the
_first_ person to increment it. And when you use "dec", and SF is set
afterwards, you know you are the _last_ person to decrement it.

That was useful when things like "xadd" weren't available, and cmpxchg
loops are expensive. So we used to have counters where -1 was that
"zero point". Very similar to your "1 is the zero point".

But was it _logical_? No. It was an implementation trick. I think
we've removed all those cases because it was so subtle and confusing
(but maybe we still have it somewhere - I did not check).

So we've certainly played those kinds of games. But it had better be
for a really good reason.

> I don't see as clear a distinction between secretmem and the above
> examples.

I really don't see what's wrong with 'atomic_t', and just checking for limits.

Saturating counters are EVIL AND BAD. They are a DoS waiting to
happen. Once you saturate, the machine is basically dead. You may have
"protected" against some attack, but you did so by killing the machine
and making the resource accounting no longer work.

So if a user can ever trigger a saturating counter, that's a big big
problem in itself.

In contrast, an 'atomic_t' with a simple limit? It just works.

And it doesn't need illogical tricks to work.

Stop thinking that refcount_t is a good type. Start realizing the
downsides. Start understanding that saturation is a HORRENDOUSLY BAD
solution, and horrible QoI.

              Linus

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Linus Torvalds]

On Mon, Oct 25, 2021 at 4:37 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Stop thinking that refcount_t is a good type. Start realizing the
> downsides. Start understanding that saturation is a HORRENDOUSLY BAD
> solution, and horrible QoI.

Basically, refcount_t should be used purely for internal kernel data
structures - it makes perfect sense for things like the 'struct
device' resource handling, for example.

In fact, there it is good for two reasons:

 - it's not counting some user resource, so users shouldn't have any
way to trigger overflow and saturation which causes problems

 - it's used by random driver stuff, which is often where kernel bugs
happen and testing is fundamentally limited by hw availability etc

but in general, anything that is user-accountable needs to have
_limits_, not saturation. It's why the page count is a "atomic_t" even
if the name of the field is "_refcount". Because refcount_t is the
INFERIOR TYPE.

Using an atomic_t properly is actually the much better option. It's
just that "properly" might be a bit more code, involving actual limit
checking.

'refcount_t' is basically a shorthand for "I didn't bother doing this
right, so I'm using this type that adds debugging, warns and stops
working and might DoS the kernel".

It's a crutch. It's not the alpha and the omega of counting types. It
has its place, but I really want to stress how people should
ABSOLUTELY not think "oh, refcount_t is better than atomic_t".

              Linus

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Kees Cook]

On Mon, Oct 25, 2021 at 04:37:01PM -0700, Linus Torvalds wrote:
> If we want other semantics, it should be a new type.

Okay, that's reasonable.

> > I see that is more like a "shared resource usage count" where the shared
> > resource doesn't necessarily disappear when we reach "no users"?
> 
> So I think that's really "atomic_t".
> 
> And instead of saturating, people should always check such shared
> resources for limits.

Right, but people make mistakes, etc. I agree about the limit being much
more sane than saturating (though in the cases of "missed decrement"),
we get to the same place: an open-coded check for the limit that never
goes down doesn't matter if it's refcount_t nor atomic_t. :)

> > i.e. there is some resource, and it starts its life with no one using it
> > (count = 1).
> 
> You are already going off into the weeds.
> 
> That's not a natural thing to do. It's already confusing. Really. Read
> that sentence yourself, and read it like an outsider.
> 
> "No one is using it, so count == 1" is a nonsensican statement on the
> face of it.
> 
> You are thinking of a refcount_t trick, not some sane semantics.
> 
> Yes, we have played off-by-one games in the kernel before. We've done
> it for various subtle reasons.

Right, sure, but it's not a rare pattern. Given that it exists, and that
it _does_ get used for allocation management (e.g. module loader), it
seems worth constructing a proper type for it so that all the open coded
stuff around these instances can be consolidated, and the API can be
defined in a way that will behave sanely.

> I really don't see what's wrong with 'atomic_t', and just checking for limits.

It's that last part. :) If we go through atomic_dec() see a zero and do
something, okay, fine. But these places need to check for insane
conditions too ("we got a -1 back -- this means there's a bug but what
do we do?"). Same for atomic_inc(): "oh, we're at our limit, do
something", but what above discovering ourselves above the limit?
There's nothing about using the atomic_t primitives that enforces these
kinds of checks. (And there likely shouldn't be for atomic_t -- it's a
plain type.) But we likely need something that fills in this API gap
between atomic_t and refcount_t.

> So if a user can ever trigger a saturating counter, that's a big big
> problem in itself.

Yes! It is. :) But they don't get to gain control over a Use-after-Free.
The risk to the system is DoS instead of loss of execution control.
That's a meaningful risk downgrade.

So, what's the right semantics for an atomic type that could be used in
the module loader, that would catch kernel counting bugs in a safe manner?

The "refcount_t but 1-based" is close, but clearly not the right name. :)

-- 
Kees Cook

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Linus Torvalds]

On Mon, Oct 25, 2021 at 5:18 PM Kees Cook <keescook@chromium.org> wrote:
>
> Right, sure, but it's not a rare pattern.

Well, for an actual reference count it certainly isn't a rare pattern,
and zero _is_ special, because at zero, you are now in use-after-free
territory.

But that's kind of the issue here: that really isn't what
'secretmem_users' was ever about.

Zero isn't some "now we're use-after-free" situation. Quite the
reverse. Zero ends up being the safe thing.

So with that kind of "just count number of existing users", where zero
isn't special, then refcount_t doesn't make sense.

And refcount_t is for non-core stuff that has a lot of random kernel
users that you can't easily verify.

In contrast, 'secretmem_users' had exactly two sites that modified it,
and one that tested it.

> But these places need to check for insane
> conditions too ("we got a -1 back -- this means there's a bug but what
> do we do?"). Same for atomic_inc(): "oh, we're at our limit, do
> something", but what above discovering ourselves above the limit?

So honestly, "above the limit" is often perfectly fine too.

It can be fine for two very different reasons:

 (a) racy checks are often much simpler and faster, and perfectly safe
when the limit is "far away from overflow".

 (b) limits can change

And (a) isn't just about "avoid special atomics". It's about doing the
limit check optimistically outside locking etc.

And (b) wasn't an issue here (where the only real use was ltierally
"are there any users at all"), but in most _proper_ use cases you will
want to have some resource limit that might be set by MIS. And might
be changed dynamically.

So it's entirely possible that somebody sets the limit to something
smaller than the current user (prep for shutdown, whatever), without
it being an error at all.

The limit is for future work, not for past work. Easily happens with
things like rlimits etc.

> There's nothing about using the atomic_t primitives that enforces these
> kinds of checks. (And there likely shouldn't be for atomic_t -- it's a
> plain type.) But we likely need something that fills in this API gap
> between atomic_t and refcount_t.

I dispute the "need". This isn't as common as you claim. Most resource
counting _is_ for "free when no longer used".

And on the other end, you have the users that don't want refcount_t
because they can't live with the limitations of that interface, like
the page counts etc, that do it properly.

So I think in 99% of all situations, the proper fix is to embed an
"atomic_t" in the type it protects, and then have the helper functions
that actually do it properly. Like we do for "get_page()" and friends.
The "new type" isn't about the reference counting, it's about the data
itself, and the atomic_t is just a part of it.

Could we do something new type that warns on the "decrement past zero"
and "overflow on increment"? Sure. But since they by _definition_
aren't about data lifetimes, they probably don't need saturation - you
want the _warning_, but they aren't protecting data, since they aren't
refcounts.

Or could we have something even fancier, that is an actual defined
range, and "overflow" is not "overflow in 32 bits", but "becomes
bigger than X")? That gets more complex because now you'd have to
encode the range in the type somehow.

You could do it with actual static types (generate typedef names and
code), or you could do it with types that have a more dynamic pointer
to ranges (kind of like the sysfs interfaces do) or have the ranges
embedded in the data structure itself.

But honestly, the complexity downside seems to just dwarf the upside.

                  Linus

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[Matthew Wilcox]

On Mon, Oct 25, 2021 at 04:37:01PM -0700, Linus Torvalds wrote:
> For example, traditionally, on x86, with atomic counting there are
> three special situations: negative, 0 and positive. So if you use the
> traditional x86 counting atomics (just add/sub/inc/dec, no xadd) then
> there are situations where you can get more information about the
> result in %eflags if you don't use zero as the initial value, but -1.
> 
> Because then you can do "inc", and if ZF is set, you know you were the
> _first_ person to increment it. And when you use "dec", and SF is set
> afterwards, you know you are the _last_ person to decrement it.
> 
> That was useful when things like "xadd" weren't available, and cmpxchg
> loops are expensive. So we used to have counters where -1 was that
> "zero point". Very similar to your "1 is the zero point".
> 
> But was it _logical_? No. It was an implementation trick. I think
> we've removed all those cases because it was so subtle and confusing
> (but maybe we still have it somewhere - I did not check).

We still do it for page->_mapcount; it's biased to -1 so that both
"the page has exactly one mapping" and "the page has no mapping" are
cheaply checkable conditions.

But, as you say, that's invisible to the users.  page_mapcount() is
return atomic_read(&page->_mapcount) + 1;

Re: [PATCH] secretmem: Prevent secretmem_users from wrapping to zero

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 4111 bytes --]

Hi "Matthew,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linux/master]
[cannot apply to hnaz-mm/master linus/master kees/for-next/pstore v5.15-rc7 next-20211029]
[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/Matthew-Wilcox-Oracle/secretmem-Prevent-secretmem_users-from-wrapping-to-zero/20211026-024503
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
config: x86_64-rhel-8.3 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/dc8e700d9fbe8484dcfa03241c44b3568d96ef4c
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Matthew-Wilcox-Oracle/secretmem-Prevent-secretmem_users-from-wrapping-to-zero/20211026-024503
        git checkout dc8e700d9fbe8484dcfa03241c44b3568d96ef4c
        # save the attached .config to linux build tree
        make W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   mm/secretmem.c: In function '__do_sys_memfd_secret':
>> mm/secretmem.c:207:18: error: passing argument 1 of 'atomic_read' from incompatible pointer type [-Werror=incompatible-pointer-types]
     207 |  if (atomic_read(&secretmem_users) < 0)
         |                  ^~~~~~~~~~~~~~~~
         |                  |
         |                  refcount_t * {aka struct refcount_struct *}
   In file included from include/linux/atomic.h:82,
                    from include/linux/cpumask.h:13,
                    from arch/x86/include/asm/cpumask.h:5,
                    from arch/x86/include/asm/msr.h:11,
                    from arch/x86/include/asm/processor.h:22,
                    from arch/x86/include/asm/cpufeature.h:5,
                    from arch/x86/include/asm/thread_info.h:53,
                    from include/linux/thread_info.h:60,
                    from arch/x86/include/asm/preempt.h:7,
                    from include/linux/preempt.h:78,
                    from include/linux/spinlock.h:55,
                    from include/linux/mmzone.h:8,
                    from include/linux/gfp.h:6,
                    from include/linux/mm.h:10,
                    from mm/secretmem.c:8:
   include/linux/atomic/atomic-instrumented.h:25:29: note: expected 'const atomic_t *' {aka 'const struct <anonymous> *'} but argument is of type 'refcount_t *' {aka 'struct refcount_struct *'}
      25 | atomic_read(const atomic_t *v)
         |             ~~~~~~~~~~~~~~~~^
   cc1: some warnings being treated as errors


vim +/atomic_read +207 mm/secretmem.c

   193	
   194	SYSCALL_DEFINE1(memfd_secret, unsigned int, flags)
   195	{
   196		struct file *file;
   197		int fd, err;
   198	
   199		/* make sure local flags do not confict with global fcntl.h */
   200		BUILD_BUG_ON(SECRETMEM_FLAGS_MASK & O_CLOEXEC);
   201	
   202		if (!secretmem_enable)
   203			return -ENOSYS;
   204	
   205		if (flags & ~(SECRETMEM_FLAGS_MASK | O_CLOEXEC))
   206			return -EINVAL;
 > 207		if (atomic_read(&secretmem_users) < 0)
   208			return -ENFILE;
   209	
   210		fd = get_unused_fd_flags(flags & O_CLOEXEC);
   211		if (fd < 0)
   212			return fd;
   213	
   214		file = secretmem_file_create(flags);
   215		if (IS_ERR(file)) {
   216			err = PTR_ERR(file);
   217			goto err_put_fd;
   218		}
   219	
   220		file->f_flags |= O_LARGEFILE;
   221	
   222		fd_install(fd, file);
   223		refcount_inc(&secretmem_users);
   224		return fd;
   225	
   226	err_put_fd:
   227		put_unused_fd(fd);
   228		return err;
   229	}
   230	

---
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: 41011 bytes --]