* Re: genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Markus Elfring @ 2019-09-27 13:15 UTC (permalink / raw)
To: Paul Moore, Navid Emamdoost, linux-security-module, netdev
Cc: Navid Emamdoost, Kangjie Lu, linux-kernel, kernel-janitors,
Stephen A McCamant, David S. Miller
In-Reply-To: <CAHC9VhR+4pZObDz7kG+rxnox2ph4z_wpZdyOL=WmdnRvdQNH9A@mail.gmail.com>
> > In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
> > allocated entry should be released.
…
> That said, netlbl_unlabel_defconf() *should* clean up here just on
> principal if nothing else.
How do you think about to add the tag “Fixes” then?
Regards,
Markus
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Linus Torvalds @ 2019-09-26 18:21 UTC (permalink / raw)
To: Micah Morton
Cc: James Morris, Jann Horn, Bart Van Assche, Paul E. McKenney,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAJ-EccMy=tNPp3=PQZxLT7eovojoAdpfQmqhAyv7XO3GwPQBMg@mail.gmail.com>
On Mon, Sep 23, 2019 at 8:31 PM Micah Morton <mortonm@chromium.org> wrote:
>
> The best way I know of ensuring this is
> for me to personally run the SafeSetID selftest (in
> tools/testing/selftests/safesetid/) every release, regardless of
> whether we make any changes to SafeSetID itself. Does this sound
> sufficient or are there more formal guidelines/processes here that I'm
> not aware of?
I think that would help, but I wopuld also hope that somebody actually
runs Chromium / Chrome OS with a modern kernel.
Even if *standard* device installs don't end up having recent kernels,
I would assume there are people who are testing development setups?
Linus
^ permalink raw reply
* Re: [PATCH] genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Paul Moore @ 2019-09-25 23:27 UTC (permalink / raw)
To: Navid Emamdoost
Cc: emamd001, kjlu, smccaman, David S. Miller, netdev,
linux-security-module, linux-kernel
In-Reply-To: <20190925221009.15523-1-navid.emamdoost@gmail.com>
On Wed, Sep 25, 2019 at 6:10 PM Navid Emamdoost
<navid.emamdoost@gmail.com> wrote:
>
> In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
> allocated entry should be released.
>
> Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
> ---
> net/netlabel/netlabel_unlabeled.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
It is worth noting that netlbl_unlabel_defconf() is only called during
kernel boot by netlbl_init() and if netlbl_unlabel_defconf() returns
an error code the system panics, so this isn't currently a very
practical concern.
That said, netlbl_unlabel_defconf() *should* clean up here just on
principal if nothing else.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
> index d2e4ab8d1cb1..c63ec480ee4e 100644
> --- a/net/netlabel/netlabel_unlabeled.c
> +++ b/net/netlabel/netlabel_unlabeled.c
> @@ -1541,8 +1541,10 @@ int __init netlbl_unlabel_defconf(void)
> entry->family = AF_UNSPEC;
> entry->def.type = NETLBL_NLTYPE_UNLABELED;
> ret_val = netlbl_domhsh_add_default(entry, &audit_info);
> - if (ret_val != 0)
> + if (ret_val != 0) {
> + kfree(entry);
> return ret_val;
> + }
>
> netlbl_unlabel_acceptflg_set(1, &audit_info);
>
> --
> 2.17.1
>
--
paul moore
www.paul-moore.com
^ permalink raw reply
* [PATCH] genetlink: prevent memory leak in netlbl_unlabel_defconf
From: Navid Emamdoost @ 2019-09-25 22:10 UTC (permalink / raw)
Cc: emamd001, kjlu, smccaman, Navid Emamdoost, Paul Moore,
David S. Miller, netdev, linux-security-module, linux-kernel
In netlbl_unlabel_defconf if netlbl_domhsh_add_default fails the
allocated entry should be released.
Signed-off-by: Navid Emamdoost <navid.emamdoost@gmail.com>
---
net/netlabel/netlabel_unlabeled.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/netlabel/netlabel_unlabeled.c b/net/netlabel/netlabel_unlabeled.c
index d2e4ab8d1cb1..c63ec480ee4e 100644
--- a/net/netlabel/netlabel_unlabeled.c
+++ b/net/netlabel/netlabel_unlabeled.c
@@ -1541,8 +1541,10 @@ int __init netlbl_unlabel_defconf(void)
entry->family = AF_UNSPEC;
entry->def.type = NETLBL_NLTYPE_UNLABELED;
ret_val = netlbl_domhsh_add_default(entry, &audit_info);
- if (ret_val != 0)
+ if (ret_val != 0) {
+ kfree(entry);
return ret_val;
+ }
netlbl_unlabel_acceptflg_set(1, &audit_info);
--
2.17.1
^ permalink raw reply related
* Re: [GIT PULL][SECURITY] Kernel lockdown patches for v5.4
From: Jiri Kosina @ 2019-09-25 14:54 UTC (permalink / raw)
To: James Morris
Cc: Linus Torvalds, linux-kernel, linux-security-module,
Matthew Garrett, David Howells
In-Reply-To: <alpine.LRH.2.21.1909101402230.20291@namei.org>
On Tue, 10 Sep 2019, James Morris wrote:
> Hi Linus,
>
> This is the latest iteration of the kernel lockdown patchset, from Matthew
> Garrett, David Howells and others.
Seems like this didn't happen (yet) ... are there any plans to either drop
it for good, or merge it?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v6 02/12] tpm-buf: add handling for TPM2B types
From: Jarkko Sakkinen @ 2019-09-25 12:34 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190925123401.GA24028@linux.intel.com>
On Wed, Sep 25, 2019 at 03:34:01PM +0300, Jarkko Sakkinen wrote:
> On Tue, Sep 24, 2019 at 07:12:40AM -0400, James Bottomley wrote:
> > I thought about that. The main problem is that most of the
> > construct/append functions use the header, and these are the functions
> > most useful to the TPM2B operation.
> >
> > The other thing that argues against this is that the TPM2B case would
> > save nothing if we eliminated the header, because we allocate a page
> > for all the data regardless.
>
> It would be way more clean. There is absolutely nothing TPM2B specific.
Given the recent regression I'm detaching allocation from tpm_buf and
make it purely a decorator (sending patch today).
/Jarkko
^ permalink raw reply
* Re: [PATCH v6 02/12] tpm-buf: add handling for TPM2B types
From: Jarkko Sakkinen @ 2019-09-25 12:34 UTC (permalink / raw)
To: James Bottomley; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <1569323560.24519.6.camel@HansenPartnership.com>
On Tue, Sep 24, 2019 at 07:12:40AM -0400, James Bottomley wrote:
> I thought about that. The main problem is that most of the
> construct/append functions use the header, and these are the functions
> most useful to the TPM2B operation.
>
> The other thing that argues against this is that the TPM2B case would
> save nothing if we eliminated the header, because we allocate a page
> for all the data regardless.
It would be way more clean. There is absolutely nothing TPM2B specific.
> > and also it makes sense to have a separate length field in the
> > struct to keep the code sane given that sometimes the buffer does not
> > store the length.
>
> I'm really not sure about that one. The header length has to be filled
> in for the non-TPM2B case but right at the moment we have no finish
> function for the buf where it could be, so we'd end up having to
> maintain two lengths in every update operation on non-TPM2B buffers.
> That seems inefficient and the only slight efficiency we get in the
> TPM2B case is not having to do the big endian conversion from the
> header which doesn't seem to be worth the added complexity.
It would be way more clean and an insignificant concern when it comes
to performance. I don't see any problem updating two lengths.
/Jarkko
^ permalink raw reply
* Re: [Patch v6 4/4] KEYS: trusted: Move TPM2 trusted keys code
From: Jarkko Sakkinen @ 2019-09-25 1:11 UTC (permalink / raw)
To: Sumit Garg
Cc: dhowells, peterhuewe, keyrings, linux-integrity,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-security-module, Herbert Xu, davem, jgg, Arnd Bergmann,
Greg Kroah-Hartman, jejb, Mimi Zohar, James Morris,
Serge E. Hallyn, Jerry Snitselaar, Linux Kernel Mailing List,
Daniel Thompson
In-Reply-To: <CAFA6WYMbUGQ6+-XvR9_qSc=oVe1QSTg4kB-+y6rBmQLq+B6skg@mail.gmail.com>
On Wed, Sep 18, 2019 at 11:53:08AM +0530, Sumit Garg wrote:
> No worries :). I will send next version of patch-set.
>
> FYI, I will be travelling for Linaro Connect next week so you could
> expect some delays in my responses.
These patches will go to v5.5. There is nothing to rush.
/Jarkko
^ permalink raw reply
* Re: Do you plan to rebase next-general any time soon?
From: Casey Schaufler @ 2019-09-24 18:35 UTC (permalink / raw)
To: James Morris; +Cc: Linux Security Module list, casey
In-Reply-To: <alpine.LRH.2.21.1909250355190.1810@namei.org>
On 9/24/2019 10:56 AM, James Morris wrote:
> On Tue, 24 Sep 2019, Casey Schaufler wrote:
>
>> Hi James. Do you have any plans to rebase next-general? I'm
>> working on the next revision of the stacking, and want to be
>> prepared for whichever way you decide to go. Thanks.
> Not unless there's a need to.
OK, thanks.
> I'd probably create a new branch (next-stacking) from v5.4 for this work.
OK, that ought to work.
^ permalink raw reply
* Re: Do you plan to rebase next-general any time soon?
From: James Morris @ 2019-09-24 17:56 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Linux Security Module list
In-Reply-To: <260cc0b6-f19e-36f7-b8e4-dad0dd0c5ded@schaufler-ca.com>
On Tue, 24 Sep 2019, Casey Schaufler wrote:
> Hi James. Do you have any plans to rebase next-general? I'm
> working on the next revision of the stacking, and want to be
> prepared for whichever way you decide to go. Thanks.
Not unless there's a need to.
I'd probably create a new branch (next-stacking) from v5.4 for this work.
--
James Morris
<jmorris@namei.org>
^ permalink raw reply
* Do you plan to rebase next-general any time soon?
From: Casey Schaufler @ 2019-09-24 17:13 UTC (permalink / raw)
To: James Morris; +Cc: Linux Security Module list
Hi James. Do you have any plans to rebase next-general? I'm
working on the next revision of the stacking, and want to be
prepared for whichever way you decide to go. Thanks.
^ permalink raw reply
* Re: [PATCH v6 05/12] tpm2-sessions: Add full HMAC and encrypt/decrypt session handling
From: James Bottomley @ 2019-09-24 11:18 UTC (permalink / raw)
To: Jarkko Sakkinen; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190920143523.GE9578@linux.intel.com>
On Fri, 2019-09-20 at 17:35 +0300, Jarkko Sakkinen wrote:
> On Fri, Sep 20, 2019 at 05:34:00PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Sep 09, 2019 at 01:20:57PM +0100, James Bottomley wrote:
>
> Forgot to ask: what is the new field handles?
You mean for the null seed or for the virtual handles?
For the former, there isn't one since the null seed is maintained as a
context when not in use, although since the null seed context is loaded
before an operation it will mostly get 80000000 for the brief time it
is used. For the latter, there's no change in the way virtual handles
are processed.
James
^ permalink raw reply
* Re: [PATCH v6 02/12] tpm-buf: add handling for TPM2B types
From: James Bottomley @ 2019-09-24 11:12 UTC (permalink / raw)
To: Jarkko Sakkinen; +Cc: linux-integrity, linux-crypto, linux-security-module
In-Reply-To: <20190920141826.GC9578@linux.intel.com>
On Fri, 2019-09-20 at 17:18 +0300, Jarkko Sakkinen wrote:
> On Mon, Sep 09, 2019 at 01:18:35PM +0100, James Bottomley wrote:
> > Most complex TPM commands require appending TPM2B buffers to the
> > command body. Since TPM2B types are essentially variable size
> > arrays,it makes it impossible to represent these complex command
> > arguments as structures and we simply have to build them up using
> > append primitives like these.
> >
> > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > om>
>
> I think a better idea would be to have headerless TPM buffers
I thought about that. The main problem is that most of the
construct/append functions use the header, and these are the functions
most useful to the TPM2B operation.
The other thing that argues against this is that the TPM2B case would
save nothing if we eliminated the header, because we allocate a page
for all the data regardless.
> and also it makes sense to have a separate length field in the
> struct to keep the code sane given that sometimes the buffer does not
> store the length.
I'm really not sure about that one. The header length has to be filled
in for the non-TPM2B case but right at the moment we have no finish
function for the buf where it could be, so we'd end up having to
maintain two lengths in every update operation on non-TPM2B buffers.
That seems inefficient and the only slight efficiency we get in the
TPM2B case is not having to do the big endian conversion from the
header which doesn't seem to be worth the added complexity.
James
> E.g.
>
> enum tpm_buf_flags {
> TPM_BUF_OVERFLOW = BIT(0),
> TPM_BUF_HEADERLESS = BIT(1),
> };
>
> struct tpm_buf {
> unsigned int length;
> struct page *data_page;
> unsigned int flags;
> u8 *data;
> };
>
> /Jarkko
>
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Micah Morton @ 2019-09-24 3:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: James Morris, Jann Horn, Bart Van Assche, Paul E. McKenney,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAHk-=wh4cuHsE8jFHO7XVatdXa=M2f4RHL3VwnSkAf5UNHUJ-Q@mail.gmail.com>
On Mon, Sep 23, 2019 at 5:45 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Sep 23, 2019 at 4:35 PM James Morris <jamorris@linuxonhyperv.com> wrote:
> >
> > My understanding is that SafeSetID is shipping in ChromeOS -- this was
> > part of the rationale for merging it.
>
> Well, if even the developer didn't test it for two months, I don't
> think "it's in upstream" makes any sense or difference.
>
> Linus
Yes, SafeSetID is shipping on Chrome OS, although I agree having that
bug in 5.3 without anyone noticing is bad. When Jann sent the last
round of patches for 5.3 he had tested the code and everything looked
good, although I unfortunately neglected to test it again after a
tweak to one of the patches, which of course broke stuff when the
patches ultimately went in.
Even though this is enabled in production for Chrome OS, none of the
Chrome OS devices are using version 5.3 yet, so it went unnoticed on
Chrome OS so far. In general the fact that a kernel feature is
shipping on Chrome OS isn't an up-to-date assurance that the feature
works in the most recent Linux release, as it would likely be months
(at least) from when a change makes it into the kernel until that
kernel release is ever run on a Chrome OS device (right now the most
recent kernel we ship on Chrome OS is 4.19, so I've had to backport
the SafeSetID stuff).
We've found this SafeSetID LSM to be pretty useful on Chrome OS, and
more use cases have popped up than we had in mind when writing it,
which suggests others would potentially find it useful as well. But I
understand for it to be useful to others it needs to be stable and
functional on every release. The best way I know of ensuring this is
for me to personally run the SafeSetID selftest (in
tools/testing/selftests/safesetid/) every release, regardless of
whether we make any changes to SafeSetID itself. Does this sound
sufficient or are there more formal guidelines/processes here that I'm
not aware of?
^ permalink raw reply
* Re: [GIT PULL] Smack patches for v5.4
From: pr-tracker-bot @ 2019-09-24 1:25 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Linus Torvalds, LKML, Linux Security Module list, casey
In-Reply-To: <d2418da0-056b-2e6f-ae40-acdfa842e341@schaufler-ca.com>
The pull request you sent on Mon, 23 Sep 2019 10:24:21 -0700:
> https://github.com/cschaufler/smack-next.git smack-for-5.4
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e94f8ccde4710f9a3e51dd3bc6134c96e33f29b3
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH] smack: include linux/watch_queue.h
From: Arnd Bergmann @ 2019-09-24 1:24 UTC (permalink / raw)
To: Casey Schaufler
Cc: James Morris, Serge E. Hallyn, David Howells, Kees Cook, Al Viro,
LSM List, linux-kernel@vger.kernel.org
In-Reply-To: <dbbf0b7a-125c-9517-4232-dd88bea139dd@schaufler-ca.com>
On Mon, Sep 23, 2019 at 2:50 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> On 9/9/2019 1:46 PM, Arnd Bergmann wrote:
> > In some randconfig builds, the lack of an explicit #include
> > in smack_lsm.c causes a build failure:
>
> What tree/branch are you working with? I don't see this.
It was in the latest linux-next at the time I posted the patch. It's
possible that
the patch causing the problem got removed in the meantime.
Arnd
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Linus Torvalds @ 2019-09-24 0:45 UTC (permalink / raw)
To: James Morris
Cc: Micah Morton, Jann Horn, Bart Van Assche, Paul E. McKenney,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1909231633400.54130@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.inter>
On Mon, Sep 23, 2019 at 4:35 PM James Morris <jamorris@linuxonhyperv.com> wrote:
>
> My understanding is that SafeSetID is shipping in ChromeOS -- this was
> part of the rationale for merging it.
Well, if even the developer didn't test it for two months, I don't
think "it's in upstream" makes any sense or difference.
Linus
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Linus Torvalds @ 2019-09-24 0:44 UTC (permalink / raw)
To: Paul E. McKenney
Cc: Micah Morton, Jann Horn, Bart Van Assche,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <20190923233038.GE7828@paulmck-ThinkPad-P72>
On Mon, Sep 23, 2019 at 4:30 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> I pushed some (untested) commits out to the dev branch of -rcu, the
> overall effect of which is shown in the patch below. The series
> adds a new rcu_replace() to avoid confusion with swap(), replaces
> uses of rcu_swap_protected() with rcu_replace(), and finally removes
> rcu_swap_protected().
>
> Is this what you had in mind?
>
> Unless you tell me otherwise, I will assume that this change is important
> but not violently urgent. (As in not for the current merge window.)
Ack, looks good to me,
Linus
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: James Morris @ 2019-09-23 23:35 UTC (permalink / raw)
To: Linus Torvalds
Cc: Micah Morton, Jann Horn, Bart Van Assche, Paul E. McKenney,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAHk-=wh_CHD9fQOyF6D2q3hVdAhFOmR8vNzcq5ZPcxKW3Nc+2Q@mail.gmail.com>
On Mon, 23 Sep 2019, Linus Torvalds wrote:
> Should we just remove safesetid again? It's not really maintained, and
> it's apparently not used. It was merged in March (with the first
> commit in January), and here we are at end of September and this
> happens.
My understanding is that SafeSetID is shipping in ChromeOS -- this was
part of the rationale for merging it.
--
James Morris
<jamorris@linuxonhyperv.com>
^ permalink raw reply
* Re: [GIT PULL] SafeSetID LSM changes for 5.4
From: Paul E. McKenney @ 2019-09-23 23:30 UTC (permalink / raw)
To: Linus Torvalds
Cc: Micah Morton, Jann Horn, Bart Van Assche,
Linux Kernel Mailing List, linux-security-module
In-Reply-To: <CAHk-=wiAsJLw1egFEE=Z7-GGtM6wcvtyytXZA1+BHqta4gg6Hw@mail.gmail.com>
On Mon, Sep 23, 2019 at 12:01:49PM -0700, Linus Torvalds wrote:
> On Wed, Sep 18, 2019 at 10:41 AM Micah Morton <mortonm@chromium.org> wrote:
> >
> > Fix for SafeSetID bug that was introduced in 5.3
>
> So this seems to be a good fix, but the bug itself came from the fact that
>
> rcu_swap_protected(..)
>
> is so hard to read, and I don't see *why* it's so pointlessly hard to read.
>
> Yes, we have some macros that change their arguments, but they have a
> _reason_ to do so (ie they return two different values) and they tend
> to be very special in other ways too.
>
> But rcu_swap_protected() has no reason for it's odd semantics.
>
> Looking at that 'handle_policy_update()' function, it's entirely
> reasonable to think "pol cannot possibly be NULL". When I looked at
> the fix patch, that was my initial reaction too, and it's probably the
> reason Jann's commit 03638e62f55f ("LSM: SafeSetID: rewrite userspace
> API to atomic updates") had that bug to begin with.
>
> I don't see the original discussion at all, it's not on
> Linux-Security-Module for some reason, so I can't tell when/if the
> NULL pointer test got deleted.
>
> Anyway, this bug would likely had been avoided if rcu_swap_protected()
> just returned the old pointer instead of changing the argument.
>
> There are only a handful or users of that macro, maybe this could be fixed?
I pushed some (untested) commits out to the dev branch of -rcu, the
overall effect of which is shown in the patch below. The series
adds a new rcu_replace() to avoid confusion with swap(), replaces
uses of rcu_swap_protected() with rcu_replace(), and finally removes
rcu_swap_protected().
Is this what you had in mind?
Unless you tell me otherwise, I will assume that this change is important
but not violently urgent. (As in not for the current merge window.)
Thanx, Paul
------------------------------------------------------------------------
diff --git a/arch/x86/kvm/pmu.c b/arch/x86/kvm/pmu.c
index 46875bb..4c37266 100644
--- a/arch/x86/kvm/pmu.c
+++ b/arch/x86/kvm/pmu.c
@@ -416,8 +416,8 @@ int kvm_vm_ioctl_set_pmu_event_filter(struct kvm *kvm, void __user *argp)
*filter = tmp;
mutex_lock(&kvm->lock);
- rcu_swap_protected(kvm->arch.pmu_event_filter, filter,
- mutex_is_locked(&kvm->lock));
+ filter = rcu_replace(kvm->arch.pmu_event_filter, filter,
+ mutex_is_locked(&kvm->lock));
mutex_unlock(&kvm->lock);
synchronize_srcu_expedited(&kvm->srcu);
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_context.c b/drivers/gpu/drm/i915/gem/i915_gem_context.c
index 0f2c22a..ebb4f15 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_context.c
@@ -1683,7 +1683,7 @@ set_engines(struct i915_gem_context *ctx,
i915_gem_context_set_user_engines(ctx);
else
i915_gem_context_clear_user_engines(ctx);
- rcu_swap_protected(ctx->engines, set.engines, 1);
+ set.engines = rcu_replace(ctx->engines, set.engines, 1);
mutex_unlock(&ctx->engines_mutex);
call_rcu(&set.engines->rcu, free_engines_rcu);
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index 1f5b5c8..6a38d4a 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -434,8 +434,8 @@ static void scsi_update_vpd_page(struct scsi_device *sdev, u8 page,
return;
mutex_lock(&sdev->inquiry_mutex);
- rcu_swap_protected(*sdev_vpd_buf, vpd_buf,
- lockdep_is_held(&sdev->inquiry_mutex));
+ vpd_buf = rcu_replace(*sdev_vpd_buf, vpd_buf,
+ lockdep_is_held(&sdev->inquiry_mutex));
mutex_unlock(&sdev->inquiry_mutex);
if (vpd_buf)
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 64c96c7..8d17779 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -466,10 +466,10 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work)
sdev->request_queue = NULL;
mutex_lock(&sdev->inquiry_mutex);
- rcu_swap_protected(sdev->vpd_pg80, vpd_pg80,
- lockdep_is_held(&sdev->inquiry_mutex));
- rcu_swap_protected(sdev->vpd_pg83, vpd_pg83,
- lockdep_is_held(&sdev->inquiry_mutex));
+ vpd_pg80 = rcu_replace(sdev->vpd_pg80, vpd_pg80,
+ lockdep_is_held(&sdev->inquiry_mutex));
+ vpd_pg83 = rcu_replace(sdev->vpd_pg83, vpd_pg83,
+ lockdep_is_held(&sdev->inquiry_mutex));
mutex_unlock(&sdev->inquiry_mutex);
if (vpd_pg83)
diff --git a/fs/afs/vl_list.c b/fs/afs/vl_list.c
index 21eb0c0..e594598 100644
--- a/fs/afs/vl_list.c
+++ b/fs/afs/vl_list.c
@@ -279,8 +279,8 @@ struct afs_vlserver_list *afs_extract_vlserver_list(struct afs_cell *cell,
struct afs_addr_list *old = addrs;
write_lock(&server->lock);
- rcu_swap_protected(server->addresses, old,
- lockdep_is_held(&server->lock));
+ old = rcu_replace(server->addresses, old,
+ lockdep_is_held(&server->lock));
write_unlock(&server->lock);
afs_put_addrlist(old);
}
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 80d6056..968d258 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -383,20 +383,22 @@ do { \
} while (0)
/**
- * rcu_swap_protected() - swap an RCU and a regular pointer
- * @rcu_ptr: RCU pointer
+ * rcu_replace() - replace an RCU pointer, returning its old value
+ * @rcu_ptr: RCU pointer, whose old value is returned
* @ptr: regular pointer
- * @c: the conditions under which the dereference will take place
+ * @c: the lockdep conditions under which the dereference will take place
*
- * Perform swap(@rcu_ptr, @ptr) where @rcu_ptr is an RCU-annotated pointer and
- * @c is the argument that is passed to the rcu_dereference_protected() call
- * used to read that pointer.
+ * Perform a replacement, where @rcu_ptr is an RCU-annotated
+ * pointer and @c is the lockdep argument that is passed to the
+ * rcu_dereference_protected() call used to read that pointer. The old
+ * value of @rcu_ptr is returned, and @rcu_ptr is set to @ptr.
*/
-#define rcu_swap_protected(rcu_ptr, ptr, c) do { \
+#define rcu_replace(rcu_ptr, ptr, c) \
+({ \
typeof(ptr) __tmp = rcu_dereference_protected((rcu_ptr), (c)); \
rcu_assign_pointer((rcu_ptr), (ptr)); \
- (ptr) = __tmp; \
-} while (0)
+ __tmp; \
+})
/**
* rcu_access_pointer() - fetch RCU pointer with no dereferencing
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 0a00eac..cee7f08 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -180,8 +180,8 @@ static void activate_effective_progs(struct cgroup *cgrp,
enum bpf_attach_type type,
struct bpf_prog_array *old_array)
{
- rcu_swap_protected(cgrp->bpf.effective[type], old_array,
- lockdep_is_held(&cgroup_mutex));
+ old_array = rcu_replace(cgrp->bpf.effective[type], old_array,
+ lockdep_is_held(&cgroup_mutex));
/* free prog array after grace period, since __cgroup_bpf_run_*()
* might be still walking the array
*/
diff --git a/net/core/dev.c b/net/core/dev.c
index fc676b2..c60454d 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1288,8 +1288,8 @@ int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
}
mutex_lock(&ifalias_mutex);
- rcu_swap_protected(dev->ifalias, new_alias,
- mutex_is_locked(&ifalias_mutex));
+ new_alias = rcu_replace(dev->ifalias, new_alias,
+ mutex_is_locked(&ifalias_mutex));
mutex_unlock(&ifalias_mutex);
if (new_alias)
diff --git a/net/core/sock_reuseport.c b/net/core/sock_reuseport.c
index 9408f92..635d202 100644
--- a/net/core/sock_reuseport.c
+++ b/net/core/sock_reuseport.c
@@ -345,8 +345,8 @@ int reuseport_detach_prog(struct sock *sk)
spin_lock_bh(&reuseport_lock);
reuse = rcu_dereference_protected(sk->sk_reuseport_cb,
lockdep_is_held(&reuseport_lock));
- rcu_swap_protected(reuse->prog, old_prog,
- lockdep_is_held(&reuseport_lock));
+ old_prog = rcu_replace(reuse->prog, old_prog,
+ lockdep_is_held(&reuseport_lock));
spin_unlock_bh(&reuseport_lock);
if (!old_prog)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 605a7cf..ea8f7cf 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -1456,8 +1456,9 @@ static void nft_chain_stats_replace(struct nft_trans *trans)
if (!nft_trans_chain_stats(trans))
return;
- rcu_swap_protected(chain->stats, nft_trans_chain_stats(trans),
- lockdep_commit_lock_is_held(trans->ctx.net));
+ nft_trans_chain_stats(trans) =
+ rcu_replace(chain->stats, nft_trans_chain_stats(trans),
+ lockdep_commit_lock_is_held(trans->ctx.net));
if (!nft_trans_chain_stats(trans))
static_branch_inc(&nft_counters_enabled);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3397122..56fe6f8 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -88,7 +88,7 @@ struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
struct tcf_chain *goto_chain)
{
a->tcfa_action = action;
- rcu_swap_protected(a->goto_chain, goto_chain, 1);
+ goto_chain = rcu_replace(a->goto_chain, goto_chain, 1);
return goto_chain;
}
EXPORT_SYMBOL(tcf_action_set_ctrlact);
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 621fb22..d4c5713 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -100,8 +100,8 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&p->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(p->params, params_new,
- lockdep_is_held(&p->tcf_lock));
+ params_new = rcu_replace(p->params, params_new,
+ lockdep_is_held(&p->tcf_lock));
spin_unlock_bh(&p->tcf_lock);
if (goto_ch)
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index b501ce0..167baaf 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -721,7 +721,7 @@ static int tcf_ct_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&c->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(c->params, params, lockdep_is_held(&c->tcf_lock));
+ params = rcu_replace(c->params, params, lockdep_is_held(&c->tcf_lock));
spin_unlock_bh(&c->tcf_lock);
if (goto_ch)
diff --git a/net/sched/act_ctinfo.c b/net/sched/act_ctinfo.c
index 10eb2bb..6d5a34d 100644
--- a/net/sched/act_ctinfo.c
+++ b/net/sched/act_ctinfo.c
@@ -256,8 +256,8 @@ static int tcf_ctinfo_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&ci->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, actparm->action, goto_ch);
- rcu_swap_protected(ci->params, cp_new,
- lockdep_is_held(&ci->tcf_lock));
+ cp_new = rcu_replace(ci->params, cp_new,
+ lockdep_is_held(&ci->tcf_lock));
spin_unlock_bh(&ci->tcf_lock);
if (goto_ch)
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 41d5398..eea4772 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -587,7 +587,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&ife->tcf_lock);
/* protected by tcf_lock when modifying existing action */
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(ife->params, p, 1);
+ p = rcu_replace(ife->params, p, 1);
if (exists)
spin_unlock_bh(&ife->tcf_lock);
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 055faa2..72ec97c 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -177,8 +177,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
goto put_chain;
}
mac_header_xmit = dev_is_mac_header_xmit(dev);
- rcu_swap_protected(m->tcfm_dev, dev,
- lockdep_is_held(&m->tcf_lock));
+ dev = rcu_replace(m->tcfm_dev, dev,
+ lockdep_is_held(&m->tcf_lock));
if (dev)
dev_put(dev);
m->tcfm_mac_header_xmit = mac_header_xmit;
diff --git a/net/sched/act_mpls.c b/net/sched/act_mpls.c
index ca2597c..e33ce9c 100644
--- a/net/sched/act_mpls.c
+++ b/net/sched/act_mpls.c
@@ -256,7 +256,7 @@ static int tcf_mpls_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&m->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(m->mpls_p, p, lockdep_is_held(&m->tcf_lock));
+ p = rcu_replace(m->mpls_p, p, lockdep_is_held(&m->tcf_lock));
spin_unlock_bh(&m->tcf_lock);
if (goto_ch)
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index a065f62..6d4817e 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -182,9 +182,9 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
police->tcfp_ptoks = new->tcfp_mtu_ptoks;
spin_unlock_bh(&police->tcfp_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(police->params,
- new,
- lockdep_is_held(&police->tcf_lock));
+ new = rcu_replace(police->params,
+ new,
+ lockdep_is_held(&police->tcf_lock));
spin_unlock_bh(&police->tcf_lock);
if (goto_ch)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 215a067..8ca1b8a 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -205,8 +205,8 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&d->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(d->params, params_new,
- lockdep_is_held(&d->tcf_lock));
+ params_new = rcu_replace(d->params, params_new,
+ lockdep_is_held(&d->tcf_lock));
spin_unlock_bh(&d->tcf_lock);
if (params_new)
kfree_rcu(params_new, rcu);
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 10dffda..a1f6ede 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -379,8 +379,8 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&t->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(t->params, params_new,
- lockdep_is_held(&t->tcf_lock));
+ params_new = rcu_replace(t->params, params_new,
+ lockdep_is_held(&t->tcf_lock));
spin_unlock_bh(&t->tcf_lock);
tunnel_key_release_params(params_new);
if (goto_ch)
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 9269d35..d5db65f 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -218,7 +218,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
spin_lock_bh(&v->tcf_lock);
goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);
- rcu_swap_protected(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
+ p = rcu_replace(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
spin_unlock_bh(&v->tcf_lock);
if (goto_ch)
diff --git a/security/safesetid/securityfs.c b/security/safesetid/securityfs.c
index d568e17..3d5ee77 100644
--- a/security/safesetid/securityfs.c
+++ b/security/safesetid/securityfs.c
@@ -179,15 +179,16 @@ static ssize_t handle_policy_update(struct file *file,
* doesn't currently exist, just use a spinlock for now.
*/
mutex_lock(&policy_update_lock);
- rcu_swap_protected(safesetid_setuid_rules, pol,
- lockdep_is_held(&policy_update_lock));
+ pol = rcu_replace(safesetid_setuid_rules, pol,
+ lockdep_is_held(&policy_update_lock));
mutex_unlock(&policy_update_lock);
err = len;
out_free_buf:
kfree(buf);
out_free_pol:
- release_ruleset(pol);
+ if (pol)
+ release_ruleset(pol);
return err;
}
^ permalink raw reply related
* Re: [PATCH] smack: include linux/watch_queue.h
From: Casey Schaufler @ 2019-09-23 21:50 UTC (permalink / raw)
To: Arnd Bergmann, James Morris, Serge E. Hallyn
Cc: David Howells, Kees Cook, Al Viro, linux-security-module,
linux-kernel
In-Reply-To: <20190909204631.1076624-1-arnd@arndb.de>
On 9/9/2019 1:46 PM, Arnd Bergmann wrote:
> In some randconfig builds, the lack of an explicit #include
> in smack_lsm.c causes a build failure:
What tree/branch are you working with? I don't see this.
>
> security/smack/smack_lsm.c:4384:7: error: incomplete definition of type 'struct watch_notification'
> if (n->type == WATCH_TYPE_META)
> ~^
> include/linux/device.h:46:8: note: forward declaration of 'struct watch_notification'
> struct watch_notification;
> ^
> security/smack/smack_lsm.c:4384:17: error: use of undeclared identifier 'WATCH_TYPE_META'
> if (n->type == WATCH_TYPE_META)
>
> Fixes: 5301fef8ca60 ("smack: Implement the watch_key and post_notification hooks [untested]")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> security/smack/smack_lsm.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a15e76489683..5120dd9c6335 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -42,6 +42,7 @@
> #include <linux/parser.h>
> #include <linux/fs_context.h>
> #include <linux/fs_parser.h>
> +#include <linux/watch_queue.h>
> #include "smack.h"
>
> #define TRANS_TRUE "TRUE"
^ permalink raw reply
* Re: [PATCH] smack: fix an compile error in smack_post_notification
From: Casey Schaufler @ 2019-09-23 21:49 UTC (permalink / raw)
To: zhong jiang; +Cc: jmorris, serge, linux-security-module, linux-kernel
In-Reply-To: <1569208607-23263-1-git-send-email-zhongjiang@huawei.com>
On 9/22/2019 8:16 PM, zhong jiang wrote:
> I hit the following error when compile the kernel.
What tree/branch are you working with? I don't see this.
>
> security/smack/smack_lsm.c: In function smack_post_notification:
> security/smack/smack_lsm.c:4383:7: error: dereferencing pointer to incomplete type struct watch_notification
> if (n->type == WATCH_TYPE_META)
> ^~
> security/smack/smack_lsm.c:4383:17: error: WATCH_TYPE_META undeclared (first use in this function); did you mean TCA_PIE_BETA?
> if (n->type == WATCH_TYPE_META)
> ^~~~~~~~~~~~~~~
> TCA_PIE_BETA
> security/smack/smack_lsm.c:4383:17: note: each undeclared identifier is reported only once for each function it appears in
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> security/smack/smack.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/security/smack/smack.h b/security/smack/smack.h
> index 62529f3..02b05a2 100644
> --- a/security/smack/smack.h
> +++ b/security/smack/smack.h
> @@ -21,6 +21,7 @@
> #include <linux/rculist.h>
> #include <linux/lsm_audit.h>
> #include <linux/msg.h>
> +#include <linux/watch_queue.h>
>
> /*
> * Use IPv6 port labeling if IPv6 is enabled and secmarks
^ permalink raw reply
* Re: [GIT PULL] Smack patches for v5.4 - retry
From: Linus Torvalds @ 2019-09-23 21:38 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Linux Security Module list, LKML
In-Reply-To: <CAHk-=wg1zkUTdnx5pNVOf=uuSJiEywNiztQe4oRiPb1pfA399w@mail.gmail.com>
On Mon, Sep 23, 2019 at 2:35 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Sep 23, 2019 at 1:14 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> >
> > Thank for the instruction. I think this is correct.
>
> Looks fine, pulled.
Oh, btw, can you get more signatures on your pgp key? I actually care
more about having a key than having a key with lots of signatures (*),
but signatures and a chain of trust would be good too.
Linus
(*) To me, keys are more of a "yeah, I'm the same person that usually
sends these pull requests" than some kind of hard identity.
^ permalink raw reply
* Re: [GIT PULL] Smack patches for v5.4 - retry
From: Linus Torvalds @ 2019-09-23 21:35 UTC (permalink / raw)
To: Casey Schaufler; +Cc: Linux Security Module list, LKML
In-Reply-To: <745ac819-f2ae-4525-1855-535daf783638@schaufler-ca.com>
On Mon, Sep 23, 2019 at 1:14 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Thank for the instruction. I think this is correct.
Looks fine, pulled.
That said, when I look closer:
> Jia-Ju Bai (1):
> security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
This one seems wrong.
Not seriously so, but the quoting the logic from the commit:
In smack_socket_sock_rcv_skb(), there is an if statement
on line 3920 to check whether skb is NULL:
if (skb && skb->secmark != 0)
This check indicates skb can be NULL in some cases.
and the fact is, skb _cannot_ be NULL, because when you test the
security of receiving an skb, you by definition always have an skb.
There is one single place that calls security_sock_rcv_skb(), and it
very much has a real skb.
So instead of adding a _new_ test for skb being NULL, the old test for
a NULL skb should just have been removed. It really doesn't make any
sense to have a NULL skb in that path - if some memory allocation had
failed on the receive path, that just means that the receive is never
done, it doesn't mean that you'd test a NULL skb for security policy
violations.
Anyway, it's pulled, but I think somebody should have checked and
thought about the automated tool reports a bit more..
Linus
^ permalink raw reply
* [GIT PULL] Smack patches for v5.4 - retry
From: Casey Schaufler @ 2019-09-23 20:14 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Linux Security Module list, LKML, casey
Hello Linus
Thank for the instruction. I think this is correct.
I have four patches for v5.4. Nothing is major. All but one are in
response to mechanically detected potential issues. The remaining
patch cleans up kernel-doc notations.
The following changes since commit 0ecfebd2b52404ae0c54a878c872bb93363ada36:
Linux 5.2 (2019-07-07 15:41:56 -0700)
are available in the Git repository at:
https://github.com/cschaufler/smack-next.git tags/smack-for-5.4-rc1
for you to fetch changes up to e5bfad3d7acc5702f32aafeb388362994f4d7bd0:
smack: use GFP_NOFS while holding inode_smack::smk_lock (2019-09-04 09:37:07 -0700)
----------------------------------------------------------------
I have four patches for v5.4. Nothing is major. All but one are in
response to mechanically detected potential issues. The remaining
patch cleans up kernel-doc notations.
----------------------------------------------------------------
Eric Biggers (1):
smack: use GFP_NOFS while holding inode_smack::smk_lock
Jann Horn (1):
Smack: Don't ignore other bprm->unsafe flags if LSM_UNSAFE_PTRACE is set
Jia-Ju Bai (1):
security: smack: Fix possible null-pointer dereferences in smack_socket_sock_rcv_skb()
luanshi (1):
smack: fix some kernel-doc notations
security/smack/smack_access.c | 6 +++---
security/smack/smack_lsm.c | 40 ++++++++++++++++++++--------------------
2 files changed, 23 insertions(+), 23 deletions(-)
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox