sksec_new->sid = sksec_sock->sid" ... which gets us back to
this function looking like a reimplementation of
selinux_sk_clone_security(), minus the peer_sid and sclass
initializations (which should be important things to have).
I strongly suggest you try making use of the existing
security_sk_clone() hook in the vsock code, it seems like a better way
to solve this problem.
--
paul moore
www.paul-moore.com
ov
> ---
> net/ipv4/cipso_ipv4.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Paul Moore
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 471d33a..6e59902 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1162
On Thu, Mar 4, 2021 at 5:33 PM David Miller wrote:
> From: Paul Moore
> Date: Thu, 04 Mar 2021 16:29:51 -0500
>
> > +static void calipso_doi_putdef(struct calipso_doi *doi_def);
> > +
>
> This is a global symbol, so why the static decl here?
To resolve this:
CC
On Thu, Mar 4, 2021 at 4:29 PM Paul Moore wrote:
>
> The current CIPSO and CALIPSO refcounting scheme for the DOI
> definitions is a bit flawed in that we:
>
> 1. Don't correctly match gets/puts in netlbl_cipsov4_list().
> 2. Decrement the refcount on each attempt to
with
refrerence counts")
Fixes: d7cce01504a0 ("netlabel: Add support for removing a CALIPSO DOI.")
Reported-by: syzbot+9ec037722d2603a9f...@syzkaller.appspotmail.com
Signed-off-by: Paul Moore
---
net/ipv4/cipso_ipv4.c| 11 +--
net/ipv6/calipso.c
On Wed, Mar 3, 2021 at 11:20 AM Paul Moore wrote:
> On Wed, Mar 3, 2021 at 10:53 AM syzbot
> wrote:
> >
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit:7a7fd0de Merge branch 'kmap-conversion-for-5.12' of git:
e 4 at addr 8880179ecb18 by task syz-executor.5/20110
Almost surely the same problem as the others, I'm currently chasing
down a few remaining spots to make sure the fix I'm working on is
correct.
--
paul moore
www.paul-moore.com
object.
> Does it make any sense?
Looking at it quickly, the logic above seems sane. I wrote this code
a *long* time ago, so let me get my head back into it and make sure
that still holds.
--
paul moore
www.paul-moore.com
ble to find one.
It's also worth adding that this code really hasn't changed much in a
*long* time, not that this means it isn't broken, just that it might
also be worth looking at other odd memory bugs to see if there is
chance they are wandering around and stomping on memory ...
--
paul moore
www.paul-moore.com
+-
> include/linux/sched.h | 7 +-
> init/init_task.c | 3 +-
> init/main.c | 2 +
> kernel/audit.c| 154 +-
> kernel/audit.h| 7 ++
> kernel/auditsc.c | 24 ++++---
> kernel/fork.c | 1 -
> 10 files changed, 205 insertions(+), 66 deletions(-)
--
paul moore
www.paul-moore.com
On Thu, Nov 19, 2020 at 10:02 PM James Morris wrote:
> On Thu, 19 Nov 2020, Paul Moore wrote:
> > As pointed out by Herbert in a recent related patch, the LSM hooks do
> > not have the necessary address family information to use the flowi
> > struct safely. As none of the L
independent flowi_common struct.
Reported-by: Herbert Xu
Signed-off-by: Paul Moore
---
.../chelsio/inline_crypto/chtls/chtls_cm.c |2 +-
drivers/net/wireguard/socket.c |4 ++-
include/linux/lsm_hook_defs.h |4 ++-
include/linux
Static checking revealed that a previous fix to
netlbl_unlabel_staticlist() leaves a stack variable uninitialized,
this patches fixes that.
Fixes: 866358ec331f ("netlabel: fix our progress tracking in
netlbl_unlabel_staticlist()")
Reported-by: Dan Carpenter
Signed-off-by: Paul Moore
ve in the kernel. This patch fixes this by ensuring
that the dump state is properly reset when necessary inside the
netlbl_unlabel_staticlist() function.
Fixes: 8cc44579d1bd ("NetLabel: Introduce static network labels for unlabeled
connections")
Signed-off-by: Paul Moore
--
On Wed, Sep 30, 2020 at 9:44 AM Paul Moore wrote:
> On Tue, Sep 29, 2020 at 7:09 PM James Morris wrote:
> > I'm not keen on adding a parameter which nobody is using. Perhaps a note
> > in the header instead?
>
> On Wed, Sep 30, 2020 at 6:14 AM Herbert Xu
> wrote:
On Fri, Oct 23, 2020 at 4:40 PM Richard Guy Briggs wrote:
> On 2020-10-22 21:21, Paul Moore wrote:
> > On Wed, Oct 21, 2020 at 12:39 PM Richard Guy Briggs wrote:
> > > Here is an exmple I was able to generate after updating the testsuite
> > > script to include a sig
+366,7 @@ static const struct netlbl_calipso_ops *calipso_ops;
>
> /**
> * netlbl_calipso_ops_register - Register the CALIPSO operations
> + * @ops: Ops to register
If we are being nitpicky, it might be better to drop the
capitalization for the sake of consistency, e.g. "@ops: ops to
registe
ot; (or similar, I'm not worried about names at this
point) to each record, reset to 0/1 at the start of each event, and
when we needed to link records somehow we could add a "related=1,..,N"
field. This would potentially be useful beyond just the audit
container ID work.
--
paul moore
www.paul-moore.com
On Mon, Oct 12, 2020 at 5:40 AM Harald Welte wrote:
>
> Hi Paul,
>
> On Sun, Oct 11, 2020 at 10:09:11PM -0400, Paul Moore wrote:
> > Harald, Pablo - I know you both suggested taking a slow iterative
> > approach to merging functionality, perhaps you could also help those
#x27;t feel like I've made much of a dent on
understanding how it is actually used.
Harald, Pablo - I know you both suggested taking a slow iterative
approach to merging functionality, perhaps you could also help those
of us on the SELinux side better understand some of the common GTP use
cases?
--
paul moore
www.paul-moore.com
cinelle.lip6.fr/):
>
> //
> @@ expression e1,e2; @@
> e1
> -,
> +;
> e2
> ... when any
> //
>
> Signed-off-by: Julia Lawall
>
> ---
> net/ipv6/calipso.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks Julia.
Acked-by: Paul Moore
&g
this is
done anywhere else in the stack so it felt wrong to do it here.
--
paul moore
www.paul-moore.com
worse.
Reported-by: Herbert Xu
Signed-off-by: Paul Moore
---
include/linux/lsm_hook_defs.h |2 +-
include/linux/lsm_hooks.h |1 +
include/linux/security.h|7 +--
net/xfrm/xfrm_state.c |4 ++--
security/security.c |5 +++--
security/se
l_flow_match hook. I checked the current code
> there and it seems to be safe for now as only secid is used which
> is part of struct flowi_common. But that API should be changed
> so that so that we don't get new bugs in the future. We could
> do that by replacing fl with just secid or adding a family field.
I'm thinking it might be better to pass the family along with the flow
instead of passing just the secid (less worry of passing an incorrect
secid that way). Let me see if I can cobble together a quick patch
for testing before bed ...
--
paul moore
www.paul-moore.com
alipso.c:605: warning: Excess function parameter 'reg'
> description in 'calipso_req_delattr'
>
> Reported-by: Hulk Robot
> Signed-off-by: Wang Hai
> ---
> net/netlabel/netlabel_calipso.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
; Signed-off-by: Wang Hai
> ---
> net/ipv4/cipso_ipv4.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Thanks for catching this and submitting the fix.
Acked-by: Paul Moore
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 2eb71579f4d2..471d33a
> security/smack/smack_lsm.c | 5 +-
> security/smack/smackfs.c| 10 ++--
> 12 files changed, 65 insertions(+), 82 deletions(-)
Minor change suggested to a comment below, but looks good otherwise.
Acked-by: Paul Moore
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv
xt);
> if (seclen)
> size += nla_total_size(seclen);
> }
I think we can get rid of the local "seclen" variable, right? We can
embed the nfqnl_get_sk_secctx() in the conditional and then simply
reference "context.len" everywhere else, yes? For example:
if (nfqnl_get_sk_secctx(..., &context))
size += nla_total_size(context.len);
--
paul moore
www.paul-moore.com
| 19 +++---
> net/netlabel/netlabel_user.c| 4 ++-
> security/security.c | 11
> 15 files changed, 121 insertions(+), 35 deletions(-)
One small comment below, but otherwise ...
Acked-by: Paul Moore
> +/**
> + * lsmcontext_init - initial
quire userspace changes to take advantage
of it, and the way forward is clearly nftables so it probably isn't
worth the effort.
I'm okay with this patch with the understanding that several chunks in
the patch are replaced by later patches in the series.
Acked-by: Paul Moore
> diff --git a
abel/netlabel_domainhash.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Thanks Alex.
Acked-by: Paul Moore
> diff --git a/net/netlabel/netlabel_domainhash.c
> b/net/netlabel/netlabel_domainhash.c
> index f73a8382c275e..dc8c39f51f7d3 100644
> --- a/net/
On Thu, Aug 27, 2020 at 1:20 PM Alex Dewar wrote:
> On Thu, Aug 27, 2020 at 06:06:34PM +0100, Alex Dewar wrote:
> > On Thu, Aug 27, 2020 at 01:00:58PM -0400, Paul Moore wrote:
> > > On Thu, Aug 27, 2020 at 12:39 PM Alex Dewar
> > > wrote:
> > > >
>
audit_log_format(audit_buf,
> -" nlbl_domain=%s res=%u",
> -entry->domain ? entry->domain : "(default)",
> -ret_val == 0 ? 1 : 0);
> +" nlbl_domain=%s",
> +entry->domain ? entry->domain : "(default)");
> audit_log_end(audit_buf);
> }
>
--
paul moore
www.paul-moore.com
On Fri, Aug 21, 2020 at 2:38 PM David Miller wrote:
> From: Paul Moore
> Date: Thu, 20 Aug 2020 21:46:14 -0400
>
> > This patch fixes two main problems seen when removing NetLabel
> > mappings: memory leaks and potentially extra audit noise.
>
> These are bug fixes the
twork address selectors to the
NetLabel/LSM domain mapping")
Reported-by: Stephen Smalley
Signed-off-by: Paul Moore
---
net/netlabel/netlabel_domainhash.c | 59 ++--
1 file changed, 30 insertions(+), 29 deletions(-)
diff --git a/net/netlabel/ne
On Fri, Aug 7, 2020 at 1:10 PM Richard Guy Briggs wrote:
> On 2020-07-05 11:11, Paul Moore wrote:
> > On Sat, Jun 27, 2020 at 9:23 AM Richard Guy Briggs wrote:
> > > Require the target task to be a descendant of the container
> > > orchestrator/engine.
If you want to
On Wed, Jul 29, 2020 at 4:06 PM Richard Guy Briggs wrote:
> On 2020-07-05 11:09, Paul Moore wrote:
> > On Sat, Jun 27, 2020 at 9:22 AM Richard Guy Briggs wrote:
...
> > > @@ -212,6 +219,33 @@ void __init audit_task_init(void)
> > >
On Wed, Jul 29, 2020 at 3:41 PM Richard Guy Briggs wrote:
> On 2020-07-05 11:10, Paul Moore wrote:
> > On Sat, Jun 27, 2020 at 9:22 AM Richard Guy Briggs wrote:
...
> > > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > > index f03d3eb0752c..9e79645e5c0e 100644
&
On Wed, Jul 29, 2020 at 3:00 PM Richard Guy Briggs wrote:
> On 2020-07-05 11:10, Paul Moore wrote:
> > On Sat, Jun 27, 2020 at 9:22 AM Richard Guy Briggs wrote:
> > >
> > > Add audit container identifier support to the action of signalling the
> > > audit
On Fri, Jul 17, 2020 at 8:44 PM Richard Guy Briggs wrote:
> On 2020-07-05 11:11, Paul Moore wrote:
> > On Sat, Jun 27, 2020 at 9:23 AM Richard Guy Briggs wrote:
> > >
> > > Add audit container identifier auxiliary record to user event standalone
> > > records
returning early from the removal
function when the entry was previously marked invalid. This change
also had the side benefit of improving the code by decreasing the
indentation level of large chunk of code by one (accounting for most
of the diffstat).
Reported-by: Stephen Smalley
Signed-off
> 1 file changed, 12 deletions(-)
Looks good to me, thanks for the patch.
Acked-by: Paul Moore
--
paul moore
www.paul-moore.com
On Mon, Jul 13, 2020 at 4:30 PM Richard Guy Briggs wrote:
> On 2020-07-07 21:42, Paul Moore wrote:
> > On Mon, Jul 6, 2020 at 10:50 PM Richard Guy Briggs wrote:
> > > On 2020-07-05 11:09, Paul Moore wrote:
> > > > On Sat, Jun 27, 2020 at 9:21 AM Rich
On Sun, Jul 12, 2020 at 7:15 PM Andrew Lunn wrote:
>
> Simple fixes which require no deep knowledge of the code.
>
> Cc: Paul Moore
> Signed-off-by: Andrew Lunn
> ---
> net/netlabel/netlabel_domainhash.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Looks go
On Sun, Jul 12, 2020 at 7:15 PM Andrew Lunn wrote:
>
> Simple fixes which require no deep knowledge of the code.
>
> Cc: Paul Moore
> Cc: Alexey Kuznetsov
> Cc: Eric Dumazet
> Signed-off-by: Andrew Lunn
> ---
> net/ipv4/cipso_ipv4.c | 6 --
> net/ipv4/ip
On Mon, Jul 6, 2020 at 10:50 PM Richard Guy Briggs wrote:
> On 2020-07-05 11:09, Paul Moore wrote:
> > On Sat, Jun 27, 2020 at 9:21 AM Richard Guy Briggs wrote:
> > >
> > > The audit-related parameters in struct task_struct should ideally be
> > > colle
properly sort out the inheritance.
> Signed-off-by: Richard Guy Briggs
> ---
> include/linux/audit.h | 1 +
> kernel/audit.c| 60
> ++-
> kernel/audit.h| 2 ++
> kernel/auditfilter.c | 17 ++-
&
on it at this point in
the patchset but thought it might be worth mentioning in case you
noticed the same and were on the fence.
--
paul moore
www.paul-moore.com
unt--;
> + if (contns->count < 1) {
One could simplify this with "(--countns->count) < 1", although if it
is changed to a refcount_t (which seems like a smart thing), the
normal decrement/test would be the best choice.
> + list_del_rcu(&contns->list);
> + kfree_rcu(contns, rcu);
> + }
> + break;
> + }
> + spin_unlock(&aunet->contobj_list_lock);
> + rcu_read_unlock();
> +}
--
paul moore
www.paul-moore.com
this might be
a better approach? My current thinking is that the capable/ns_capable
approach is preferable as it leverages existing kernel mechanisms and
doesn't require us to reinvent the wheel in the audit subsystem.
--
paul moore
www.paul-moore.com
ntext = audit_alloc_context(AUDIT_RECORD_CONTEXT, gfpflags);
> + if (!context) {
> + audit_log_lost("out of memory in audit_alloc_local");
> + goto out;
You might as well just return NULL here, no need to jump and then return NULL.
> + }
o add to the capability check. Patch 2 adds a
"capable(CAP_AUDIT_CONTROL)" which is good, but perhaps we also need a
"ns_capable(CAP_AUDIT_CONTROL)" to allow a given audit container ID
orchestrator/owner the ability to control which of it's descendants
can change their audit container ID, for example:
if (!capable(CAP_AUDIT_CONTROL) ||
!ns_capable(current->nsproxy->user_ns, CAP_AUDIT_CONTROL))
return -EPERM;
--
paul moore
www.paul-moore.com
goto conterror;
> + }
> _audit_contobj_hold(cont);
> newcont = cont;
> } else {
> diff --git a/security/selinux/nlmsgtab.c b/security/selinux/nlmsgtab.c
> index b69231918686..8303bb7a63d0 100644
> --- a/security/selinux/nlmsgtab.c
> +++ b/security/selinux/nlmsgtab.c
> @@ -137,6 +137,7 @@ struct nlmsg_perm {
> { AUDIT_DEL_RULE, NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
> { AUDIT_USER, NETLINK_AUDIT_SOCKET__NLMSG_RELAY},
> { AUDIT_SIGNAL_INFO,NETLINK_AUDIT_SOCKET__NLMSG_READ },
> + { AUDIT_SIGNAL_INFO2, NETLINK_AUDIT_SOCKET__NLMSG_READ },
> { AUDIT_TRIM, NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
> { AUDIT_MAKE_EQUIV, NETLINK_AUDIT_SOCKET__NLMSG_WRITE},
> { AUDIT_TTY_GET,NETLINK_AUDIT_SOCKET__NLMSG_READ },
--
paul moore
www.paul-moore.com
d.mnt) {
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_CWD);
> @@ -1575,6 +1590,14 @@ static void audit_log_exit(void)
>
> audit_log_proctitle();
>
> + rcu_read_lock();
> + cont = _audit_contobj_get(current);
> + rcu_read_unlock();
> + audit_log_container_id(context, cont);
> + rcu_read_lock();
> + _audit_contobj_put(cont);
> + rcu_read_unlock();
Do we need to grab an additional reference for the audit container
object here? We don't create any additional references here that
persist beyond the lifetime of this function, right?
> audit_log_container_drop();
>
> /* Send end of event record to help user space know we are finished */
--
paul moore
www.paul-moore.com
_log_exit(void)
>
> audit_log_proctitle();
>
> + audit_log_container_drop();
> +
> /* Send end of event record to help user space know we are finished */
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_EOE);
> if (ab)
--
paul moore
www.paul-moore.com
tsk->audit = NULL;
> + kmem_cache_free(audit_task_cache, info);
Another nitpick, and this one may even become a moot point given the
question posed above. However, is there any reason we couldn't get
rid of "info" and simplify this a bit?
audit_free_syscall(tsk);
kmem_cache_free(audit_task_cache, tsk->audit);
tsk->audit = NULL;
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 468a23390457..f00c1da587ea 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1612,7 +1615,6 @@ void __audit_free(struct task_struct *tsk)
> if (context->current_state == AUDIT_RECORD_CONTEXT)
> audit_log_exit();
> }
> -
> audit_set_context(tsk, NULL);
> audit_free_context(context);
> }
This nitpick is barely worth the time it is taking me to write this,
but the whitespace change above isn't strictly necessary.
--
paul moore
www.paul-moore.com
list_add_rcu(&newcont->list,
> + &audit_contid_hash[h]);
> + } else {
> + rc = -ENOMEM;
> + spin_unlock(&audit_contobj_list_lock);
> + goto conterror;
> + }
> + }
> + spin_unlock(&audit_contobj_list_lock);
> + task->audit->cont = newcont;
> + _audit_contobj_put(oldcont);
> + }
> +conterror:
> + task_unlock(task);
> +
> + if (!audit_enabled)
> + return rc;
> +
> + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONTAINER_OP);
> + if (!ab)
> + return rc;
> +
> + audit_log_format(ab,
> +"op=set opid=%d contid=%llu old-contid=%llu",
> +task_tgid_nr(task), contid, oldcont ? oldcont->id :
> -1);
> + _audit_contobj_put(oldcont);
> + rcu_read_unlock();
> + audit_log_end(ab);
> + return rc;
> +}
--
paul moore
www.paul-moore.com
On Sat, Jul 4, 2020 at 9:29 AM Paul Moore wrote:
> On Sat, Jun 27, 2020 at 9:22 AM Richard Guy Briggs wrote:
> >
> > Implement the proc fs write to set the audit container identifier of a
> > process, emitting an AUDIT_CONTAINER_OP record to document the event.
Sorry ab
> + } else {
> + rc = -ENOMEM;
> + spin_unlock(&audit_contobj_list_lock);
> + goto conterror;
> + }
> + }
> + spin_unlock(&audit_contobj_list_lock);
> + task->audit->cont = newcont;
> + _audit_contobj_put(oldcont);
> + }
> +conterror:
> + task_unlock(task);
> +
> + if (!audit_enabled)
> + return rc;
> +
> + ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONTAINER_OP);
> + if (!ab)
> + return rc;
> +
> + audit_log_format(ab,
> +"op=set opid=%d contid=%llu old-contid=%llu",
> +task_tgid_nr(task), contid, oldcont ? oldcont->id :
> -1);
> + _audit_contobj_put(oldcont);
> + rcu_read_unlock();
> + audit_log_end(ab);
> + return rc;
> +}
> +
> /**
> * audit_log_end - end one audit record
> * @ab: the audit_buffer
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 9bee09757068..182fc76ea276 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -210,6 +210,14 @@ static inline int audit_hash_ino(u32 ino)
> return (ino & (AUDIT_INODE_BUCKETS-1));
> }
>
> +#define AUDIT_CONTID_BUCKETS 32
> +extern struct list_head audit_contid_hash[AUDIT_CONTID_BUCKETS];
> +
> +static inline int audit_hash_contid(u64 contid)
> +{
> + return (contid & (AUDIT_CONTID_BUCKETS-1));
> +}
> +
> /* Indicates that audit should log the full pathname. */
> #define AUDIT_NAME_FULL -1
>
> --
> 1.8.3.1
>
--
paul moore
www.paul-moore.com
On Mon, Jun 8, 2020 at 2:04 PM Richard Guy Briggs wrote:
> On 2020-04-22 13:24, Paul Moore wrote:
> > On Fri, Apr 17, 2020 at 6:26 PM Eric W. Biederman
> > wrote:
> > > Paul Moore writes:
> > > > On Thu, Apr 16, 2020 at 4:36 PM Eric W. Biederman
> &g
Abeni
> ---
> net/ipv4/cipso_ipv4.c| 6 --
> net/ipv6/calipso.c | 3 ++-
> net/netlabel/netlabel_kapi.c | 6 ++
> 3 files changed, 12 insertions(+), 3 deletions(-)
Seems reasonable to me, thanks Paolo.
Acked-by: Paul Moore
--
paul moore
www.paul-moore.com
On Thu, Aug 29, 2019 at 3:45 AM Michal Kubecek wrote:
> On Tue, Aug 27, 2019 at 04:47:04PM -0400, Paul Moore wrote:
> >
> > I'm also not a big fan of inserting the hook in rtnl_fill_ifinfo(); as
> > presented it is way too specific for a LSM hook for me to be happy.
&g
l a matter
of where to place the hook.
If we only care about netlink messages which leverage nlattrs I
suppose one option that I haven't seen mentioned would be to place a
hook in nla_put(). While it is a bit of an odd place for a hook, it
would allow the LSM easy access to the skb and attribute type to make
decisions, and all of the callers should already be checking the
return code (although we would need to verify this). One notable
drawback (not the only one) is that the hook is going to get hit
multiple times for each message.
--
paul moore
www.paul-moore.com
On Thu, Aug 22, 2019 at 3:03 AM Florian Westphal wrote:
> Paul Moore wrote:
> > Hello netdev,
> >
> > I was just made aware of the skb extension work, and it looks very
> > appealing from a LSM perspective. As some of you probably remember,
> > we (the LSM fol
On Wed, Aug 21, 2019 at 6:50 PM David Miller wrote:
> From: Paul Moore
> Date: Wed, 21 Aug 2019 18:00:09 -0400
>
> > I was just made aware of the skb extension work, and it looks very
> > appealing from a LSM perspective. As some of you probably remember,
> > we (t
were to propose a patchset to add a SKB_EXT_SECURITY skb
extension (a single extension ID to be shared among the different
LSMs), would that be something that netdev would consider merging, or
is there still a philosophical objection to things like this?
--
paul moore
www.paul-moore.com
On Mon, Jul 8, 2019 at 2:06 PM Richard Guy Briggs wrote:
> On 2019-05-30 15:29, Paul Moore wrote:
...
> > [REMINDER: It is an "*audit* container ID" and not a general
> > "container ID" ;) Smiley aside, I'm not kidding about that part.]
> >
the function and jump there on error.
> }
> rc = selinux_add_opt(token, arg, mnt_opts);
> if (unlikely(rc)) {
--
paul moore
www.paul-moore.com
On Thu, May 9, 2019 at 4:40 AM Paolo Abeni wrote:
> On Wed, 2019-05-08 at 17:17 -0400, Paul Moore wrote:
> > On Wed, May 8, 2019 at 2:55 PM Stephen Smalley wrote:
> > > On 5/8/19 2:27 PM, Marcelo Ricardo Leitner wrote:
> > > > On Wed, May 08, 2019 at 02:13:17P
t; somehow re-use shutdown() stuff? It gets very confusing, and after
> > all, it still is, in essence, a connect() syscall.
>
> The function checks CONNECT permission on entry, before reaching this
> point. This logic is only in preparation for a further check
> (NAME_CONNECT) on the port. In this case, there is no further check to
> perform and we can just return.
I agree with Stephen, in the connect(AF_UNSPEC) case the right thing
to do is to simply return with no error.
I would also suggest that since this patch only touches the SELinux
code it really should go in via the SELinux tree and not netdev; this
will help avoid merge conflicts in the linux-next tree and during the
merge window. I think the right thing to do at this point is to
create a revert patch (or have DaveM do it, I'm not sure what he
prefers in situations like this) for this commit, make the adjustments
that Stephen mentioned and submit them for the SELinux tree.
Otherwise, thanks for catching this and submitting a fix :)
--
paul moore
www.paul-moore.com
++-
> security/apparmor/lsm.c | 6 ++
> security/selinux/hooks.c| 12 ++++
> security/smack/smack_lsm.c | 39 +++
> security/tomoyo/network.c | 12
> 14 files changed, 85 insertions(+), 13 deletions(-)
Some minor ni
On Tue, Apr 9, 2019 at 9:49 AM Neil Horman wrote:
> On Tue, Apr 09, 2019 at 09:40:58AM -0400, Paul Moore wrote:
> > On Tue, Apr 9, 2019 at 8:58 AM Ondrej Mosnacek wrote:
> > >
> > > On Tue, Apr 9, 2019 at 5:40 AM Richard Guy Briggs wrote:
> > > > Add au
On Fri, Mar 8, 2019 at 9:47 PM Paul Moore wrote:
> On Fri, Mar 8, 2019 at 12:08 PM Marcelo Ricardo Leitner
> wrote:
> > On Sat, Mar 09, 2019 at 12:07:34AM +0800, Xin Long wrote:
> > > As does in __sctp_connect(), when checking addrs in a while loop, after
> > >
m -net trees.
For patches that warrant inclusion in -stable, I merge them into the
current selinux/stable-X.Y and send it up to Linus once it passes some
basic sanity tests. Since we're still only half-way through the merge
window, and this is an obvious bug fix, I think this qualifies as
-stable material.
I'm traveling at the moment with not-so-great connectivity, but I'll
get this merged and verified early next week.
--
paul moore
www.paul-moore.com
On Fri, Mar 8, 2019 at 1:31 AM maowenan wrote:
> On 2019/3/8 4:36, Paul Moore wrote:
> > On Wed, Mar 6, 2019 at 9:44 PM Mao Wenan wrote:
> >>
> >> If netlbl_sock_setattr() is failed, it directly returns rc and forgets
> >> to free secattr.
> >>
&
> + if (rc != 0) {
> + netlbl_secattr_free(secattr);
> + }
This is likely going to cause a problem as the
sock->sk_security->nlbl_secattr still has a reference to the secattr
pointer you are releasing here. Assuming things are working correctly
elsewhere, I believe freeing secattr here will result in a double free
when the network stacks cleans up after the failed socket creation
(via the sock_release() call in the error handling code).
It looks like you may have found this via a test tool (syzbot?), do
you have a reproducer you can share?
--
paul moore
www.paul-moore.com
y the netlbl_bitmap_walk() patch to
cipso_v4_bitmap_walk() as netlbl_bitmap_walk() doesn't exist before
Linux v4.8.
Reported-by: Jann Horn
Fixes: 446fda4f2682 ("[NetLabel]: CIPSOv4 engine")
Fixes: 3faa8f982f95 ("netlabel: Move bitmap manipulation functions to the
NetLabel core.&qu
g things to threads sometimes, and obscured the subject
line), however, I did look at the content of patch before giving it my
thumbs up. Claiming the email wasn't read isn't correct (although you
could rightly argue I didn't read the subject line), or I'm "rubber
stamping crap" isn't correct.
--
paul moore
www.paul-moore.com
sertions(+), 4 deletions(-)
Reviewed-by: Paul Moore
> diff --git a/include/net/icmp.h b/include/net/icmp.h
> index 6ac3a5b..e0f709d 100644
> --- a/include/net/icmp.h
> +++ b/include/net/icmp.h
> @@ -22,6 +22,7 @@
>
> #include
> #include
> +#include
>
> str
ipv4/ip_options.c | 22 +-
> 3 files changed, 34 insertions(+), 7 deletions(-)
Thanks Sergey.
Acked-by: Paul Moore
> diff --git a/include/net/ip.h b/include/net/ip.h
> index 8866bfc..f0e8d06 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -667,6 +667,8 @@ static
On Fri, Feb 15, 2019 at 3:00 PM David Miller wrote:
> From: Paul Moore
> Date: Fri, 15 Feb 2019 14:02:31 -0500
>
> > On Thu, Feb 14, 2019 at 11:43 AM David Miller wrote:
> >> From: Nazarov Sergey
> >> Date: Tue, 12 Feb 2019 18:10:03 +0300
> >>
> &
tree, there are probably a couple more
> locations as well.
David, are you happy with Sergey's solution as a fix for this?
If so, would you prefer a respin of this patch to apply the to the
other broken callers (e.g. net/atm/clip.c), or would you rather merge
this patch and deal with the other callers in separate patches?
--
paul moore
www.paul-moore.com
mark);
>
> - if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
> + if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in,
> opt))
> goto out_unlock;
>
>
> @@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int
> code, __be32 info)
> local_bh_enable();
> out:;
> }
> -EXPORT_SYMBOL(icmp_send);
> +EXPORT_SYMBOL(__icmp_send);
>
>
> static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
> --
>
--
paul moore
www.paul-moore.com
ailing list archives to see what others have done in terms of
commit descriptions, etc.
After that, if you have any questions let me know and I can help you out.
Thanks.
> 11.02.2019, 23:37, "Paul Moore" :
> > On Thu, Jan 31, 2019 at 8:20 AM Nazarov Sergey wrote:
> &
On Thu, Jan 31, 2019 at 8:20 AM Nazarov Sergey wrote:
> 31.01.2019, 05:10, "Paul Moore" :
> > This isn't how the rest of the stack works, look at
> > ip_local_deliver_finish() for one example. Perhaps the behavior you
> > are proposing is correct, but ple
On Fri, Feb 1, 2019 at 1:26 AM Dmitry Vyukov wrote:
> On Wed, Jan 30, 2019 at 10:30 PM Paul Moore wrote:
> >
> > On Wed, Jan 30, 2019 at 4:01 PM syzbot
> > wrote:
> > >
> > > Hello,
> > >
> > > syzbot found the following crash on:
On Wed, Jan 30, 2019 at 8:11 AM Nazarov Sergey wrote:
> 30.01.2019, 01:42, "Paul Moore" :
> > There are several cases where the stack ends up calling icmp_send()
> > after the skb has been through ip_options_compile(), that should be
> > okay.
> >
> &g
On Tue, Jan 29, 2019 at 2:23 AM Nazarov Sergey wrote:
> 29.01.2019, 01:18, "Paul Moore" :
> > If we don't pass a skb into ip_options_compile(), meaning both "skb"
> > and "rt" will be NULL, then I don't believe the option data will
> >
On Mon, Jan 28, 2019 at 8:10 AM Nazarov Sergey wrote:
> 25.01.2019, 19:46, "Paul Moore" :
> > Hmm, I think the above calculation should take into account the actual
> > length of the IP options, and not just the max size (calculate it
> > based on iphdr->ihl)
On Thu, Jan 24, 2019 at 9:46 AM Nazarov Sergey wrote:
> 22.01.2019, 20:48, "Paul Moore" :
> >
> > Yes, exactly. If you don't pass the skb it shouldn't attempt to call
> > icmp_send() in case of error.
> >
> > --
> > paul moore
>
On Tue, Jan 22, 2019 at 12:35 PM Nazarov Sergey wrote:
> 22.01.2019, 19:49, "Paul Moore" :
> >
> > Granted I'm looking at this rather quickly, so I may be missing
> > something, but why the changes to ip_options_compile()? Couldn't you
> >
On Mon, Jan 21, 2019 at 12:11 PM Nazarov Sergey wrote:
> 18.01.2019, 20:17, "Paul Moore" :
> > Yes, this is extra overhead, but it is in an error path so I'm not
> > overly concerned about the performance impact on the given connection.
> >
> > I will ad
gt; May be better to add some flag or pointer to struct sk_buff?
> But, I think, we need netdev developers advice for this.
> Regards, Sergey.
>
> 18.01.2019, 17:53, "Paul Moore" :
> > On Tue, Jan 15, 2019 at 2:52 PM Paul Moore wrote:
> >
> > It's been a
On Tue, Jan 15, 2019 at 2:52 PM Paul Moore wrote:
> On Tue, Jan 15, 2019 at 12:55 PM Casey Schaufler
> wrote:
> > On 1/15/2019 9:06 AM, Nazarov Sergey wrote:
> > > Hello!
> > > Security modules (selinux, smack) use icmp_send for discarded incorrectly
> > &
gt; + if (__ip_options_echo(&icmp_param->replyopts.opt.opt, skb_in,
> > + ip_hdr(skb_in)->protocol == IPPROTO_TCP ?
> > &TCP_SKB_CB(skb_in)->header.h4.opt : &IPCB(skb_in)->opt))
> > goto out_unlock;
> >
> >
--
paul moore
www.paul-moore.com
with other upstream review to get other angles and to
> take some of the load and responsibility off the primary maintainer.
>
> I expect to submit a v5 within a week without having had those questions
> directly answered, but with some ideas of what to check and verify
> before I resubmit. Most of the changes have been sitting in that branch
> for two months, already rebased one kernel version and will need
> updating again.
--
paul moore
www.paul-moore.com
On Thu, Dec 27, 2018 at 10:34 AM Richard Guy Briggs wrote:
> On 2018-10-31 15:30, Richard Guy Briggs wrote:
> > On 2018-10-19 19:18, Paul Moore wrote:
> > > On Sun, Aug 5, 2018 at 4:33 AM Richard Guy Briggs wrote:
> > > > Add audit container identifier auxili
On Wed, Nov 28, 2018 at 1:44 PM Paul Moore wrote:
> Commit 32a4f5ecd738 ("net: sched: introduce chain object to uapi")
> added new RTM_* definitions without properly updating SELinux, this
> patch adds the necessary SELinux support.
>
> While there was a BUILD_BUG_ON
the broken commit. In order to
hopefully prevent this from happening in the future, add additional
comments which provide some instructions on how to resolve the
BUILD_BUG_ON() failures.
Fixes: 32a4f5ecd738 ("net: sched: introduce chain object to uapi")
Cc: # 4.19
Signed-off-by: Paul
ed, 7 insertions(+), 4 deletions(-)
See my previous comments about the necessity of this patch, but beyond
that it looks fine to me.
Acked-by: Paul Moore
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 82178cc69c96..777fa3b7fb13 100644
> --- a/net/ipv4/cipso_i
1 - 100 of 639 matches
Mail list logo
