* Re: [net-next V8 PATCH 1/5] bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
From: Daniel Borkmann @ 2017-10-17 14:00 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Alexei Starovoitov
Cc: netdev, jakub.kicinski, Michael S. Tsirkin, pavel.odintsov,
Jason Wang, mchan, John Fastabend, peter.waskiewicz.jr, ast,
Daniel Borkmann, Andy Gospodarek
In-Reply-To: <20171017124729.59e45c74@redhat.com>
On 10/17/2017 12:47 PM, Jesper Dangaard Brouer wrote:
> On Mon, 16 Oct 2017 14:49:53 -0700
> Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>
>> On Mon, Oct 16, 2017 at 12:19:28PM +0200, Jesper Dangaard Brouer wrote:
>>> The 'cpumap' is primarily used as a backend map for XDP BPF helper
>>> call bpf_redirect_map() and XDP_REDIRECT action, like 'devmap'.
>>>
>>> This patch implement the main part of the map. It is not connected to
>>> the XDP redirect system yet, and no SKB allocation are done yet.
>>>
>>> The main concern in this patch is to ensure the datapath can run
>>> without any locking. This adds complexity to the setup and tear-down
>>> procedure, which assumptions are extra carefully documented in the
>>> code comments.
>>>
>>> V2:
>>> - make sure array isn't larger than NR_CPUS
>>> - make sure CPUs added is a valid possible CPU
>>>
>>> V3: fix nitpicks from Jakub Kicinski <kubakici@wp.pl>
>>>
>>> V5:
>>> - Restrict map allocation to root / CAP_SYS_ADMIN
>>> - WARN_ON_ONCE if queue is not empty on tear-down
>>> - Return -EPERM on memlock limit instead of -ENOMEM
>>> - Error code in __cpu_map_entry_alloc() also handle ptr_ring_cleanup()
>>> - Moved cpu_map_enqueue() to next patch
>>>
>>> V6: all notice by Daniel Borkmann
>>> - Fix err return code in cpu_map_alloc() introduced in V5
>>> - Move cpu_possible() check after max_entries boundary check
>>> - Forbid usage initially in check_map_func_compatibility()
>>>
>>> V7:
>>> - Fix alloc error path spotted by Daniel Borkmann
>>> - Did stress test adding+removing CPUs from the map concurrently
>>> - Fixed refcnt issue on cpu_map_entry, kthread started too soon
>>> - Make sure packets are flushed during tear-down, involved use of
>>> rcu_barrier() and kthread_run only exit after queue is empty
>>> - Fix alloc error path in __cpu_map_entry_alloc() for ptr_ring
>>>
>>> V8:
>>> - Nitpicking comments and gramma by Edward Cree
>>> - Fix missing semi-colon introduced in V7 due to rebasing
>>> - Move struct bpf_cpu_map_entry members cpu+map_id to tracepoint patch
>>>
>>> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
>>> ---
>>> include/linux/bpf_types.h | 1
>>> include/uapi/linux/bpf.h | 1
>>> kernel/bpf/Makefile | 1
>>> kernel/bpf/cpumap.c | 560 ++++++++++++++++++++++++++++++++++++++++
>>> kernel/bpf/syscall.c | 8 +
>>> kernel/bpf/verifier.c | 5
>>> tools/include/uapi/linux/bpf.h | 1
>>> 7 files changed, 576 insertions(+), 1 deletion(-)
>>> create mode 100644 kernel/bpf/cpumap.c
>>
>> Looks good to me
>> I like the idea of running networking stack from kthread
>> and hope adding GRO won't change the api.
>> Acked-by: Alexei Starovoitov <ast@kernel.org>
>
> Thanks!
>
> I think adding GRO is still safe API-wise after this patchset. I
> imagine that the GRO API will be tied to how we implement/expose the
> RX-hash to the eBPF program. Thus, the API will be the eBPF prog can
> change the RX-hash, to influence the GRO aggregation/partial-sort. If
> the map need to behave differently for GRO then we have the map_flags
> to adjust this behavior (but I assume this would not be needed).
+1, this should happen transparent to the user. But would it mean that
the cpumap threads get a fake napi_struct for napi_gro_receive() or
would it require larger refactoring/splitting of gro engine internals?
Would be good to have a clearer picture on that before uapi freezes.
Thanks,
Daniel
^ permalink raw reply
* [RFC PATCH 2/5] sctp: Add ip option support
From: Richard Haines @ 2017-10-17 13:58 UTC (permalink / raw)
To: selinux, netdev, linux-sctp, linux-security-module
Cc: paul, vyasevich, nhorman, sds, eparis, marcelo.leitner,
Richard Haines
Add ip option support to allow LSM security modules to utilise CIPSO/IPv4
and CALIPSO/IPv6 services.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
include/net/sctp/structs.h | 2 ++
net/sctp/chunk.c | 7 ++++---
net/sctp/ipv6.c | 37 ++++++++++++++++++++++++++++++-------
net/sctp/output.c | 3 ++-
net/sctp/protocol.c | 36 ++++++++++++++++++++++++++++++++++++
net/sctp/socket.c | 5 ++++-
6 files changed, 78 insertions(+), 12 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 5ab29af..7767577 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -461,6 +461,7 @@ struct sctp_af {
void (*ecn_capable)(struct sock *sk);
__u16 net_header_len;
int sockaddr_len;
+ int (*ip_options_len)(struct sock *sk);
sa_family_t sa_family;
struct list_head list;
};
@@ -485,6 +486,7 @@ struct sctp_pf {
int (*addr_to_user)(struct sctp_sock *sk, union sctp_addr *addr);
void (*to_sk_saddr)(union sctp_addr *, struct sock *sk);
void (*to_sk_daddr)(union sctp_addr *, struct sock *sk);
+ void (*copy_ip_options)(struct sock *sk, struct sock *newsk);
struct sctp_af *af;
};
diff --git a/net/sctp/chunk.c b/net/sctp/chunk.c
index 1323d41..e49e240 100644
--- a/net/sctp/chunk.c
+++ b/net/sctp/chunk.c
@@ -153,7 +153,6 @@ static void sctp_datamsg_assign(struct sctp_datamsg *msg, struct sctp_chunk *chu
chunk->msg = msg;
}
-
/* A data chunk can have a maximum payload of (2^16 - 20). Break
* down any such message into smaller chunks. Opportunistically, fragment
* the chunks down to the current MTU constraints. We may get refragmented
@@ -190,7 +189,10 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
*/
max_data = asoc->pathmtu -
sctp_sk(asoc->base.sk)->pf->af->net_header_len -
- sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk);
+ sizeof(struct sctphdr) - sizeof(struct sctp_data_chunk) -
+ sctp_sk(asoc->base.sk)->pf->af->
+ ip_options_len(asoc->base.sk);
+
max_data = SCTP_TRUNC4(max_data);
/* If the the peer requested that we authenticate DATA chunks
@@ -210,7 +212,6 @@ struct sctp_datamsg *sctp_datamsg_from_user(struct sctp_association *asoc,
/* Set first_len and then account for possible bundles on first frag */
first_len = max_data;
-
/* Check to see if we have a pending SACK and try to let it be bundled
* with this message. Do this if we don't have any data queued already.
* To check that, look at out_qlen and retransmit list.
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index a4b6ffb..49c9011 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -423,6 +423,33 @@ static void sctp_v6_copy_addrlist(struct list_head *addrlist,
rcu_read_unlock();
}
+/* Copy over any ip options */
+static void sctp_v6_copy_ip_options(struct sock *sk, struct sock *newsk)
+{
+ struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
+ struct ipv6_txoptions *opt;
+
+ newnp = inet6_sk(newsk);
+
+ rcu_read_lock();
+ opt = rcu_dereference(np->opt);
+ if (opt)
+ opt = ipv6_dup_options(newsk, opt);
+ RCU_INIT_POINTER(newnp->opt, opt);
+ rcu_read_unlock();
+}
+
+/* Account for the IP options */
+static int sctp_v6_ip_options_len(struct sock *sk)
+{
+ struct ipv6_pinfo *inet6 = inet6_sk(sk);
+
+ if (inet6->opt)
+ return inet6->opt->opt_flen + inet6->opt->opt_nflen;
+ else
+ return 0;
+}
+
/* Initialize a sockaddr_storage from in incoming skb. */
static void sctp_v6_from_skb(union sctp_addr *addr, struct sk_buff *skb,
int is_saddr)
@@ -662,7 +689,6 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
struct sock *newsk;
struct ipv6_pinfo *newnp, *np = inet6_sk(sk);
struct sctp6_sock *newsctp6sk;
- struct ipv6_txoptions *opt;
newsk = sk_alloc(sock_net(sk), PF_INET6, GFP_KERNEL, sk->sk_prot, kern);
if (!newsk)
@@ -685,12 +711,7 @@ static struct sock *sctp_v6_create_accept_sk(struct sock *sk,
newnp->ipv6_ac_list = NULL;
newnp->ipv6_fl_list = NULL;
- rcu_read_lock();
- opt = rcu_dereference(np->opt);
- if (opt)
- opt = ipv6_dup_options(newsk, opt);
- RCU_INIT_POINTER(newnp->opt, opt);
- rcu_read_unlock();
+ sctp_v6_copy_ip_options(sk, newsk);
/* Initialize sk's sport, dport, rcv_saddr and daddr for getsockname()
* and getpeername().
@@ -1033,6 +1054,7 @@ static struct sctp_af sctp_af_inet6 = {
.ecn_capable = sctp_v6_ecn_capable,
.net_header_len = sizeof(struct ipv6hdr),
.sockaddr_len = sizeof(struct sockaddr_in6),
+ .ip_options_len = sctp_v6_ip_options_len,
#ifdef CONFIG_COMPAT
.compat_setsockopt = compat_ipv6_setsockopt,
.compat_getsockopt = compat_ipv6_getsockopt,
@@ -1051,6 +1073,7 @@ static struct sctp_pf sctp_pf_inet6 = {
.addr_to_user = sctp_v6_addr_to_user,
.to_sk_saddr = sctp_v6_to_sk_saddr,
.to_sk_daddr = sctp_v6_to_sk_daddr,
+ .copy_ip_options = sctp_v6_copy_ip_options,
.af = &sctp_af_inet6,
};
diff --git a/net/sctp/output.c b/net/sctp/output.c
index 9d85049..85bcd5b 100644
--- a/net/sctp/output.c
+++ b/net/sctp/output.c
@@ -151,7 +151,8 @@ void sctp_packet_init(struct sctp_packet *packet,
INIT_LIST_HEAD(&packet->chunk_list);
if (asoc) {
struct sctp_sock *sp = sctp_sk(asoc->base.sk);
- overhead = sp->pf->af->net_header_len;
+ overhead = sp->pf->af->net_header_len +
+ sp->pf->af->ip_options_len(asoc->base.sk);
} else {
overhead = sizeof(struct ipv6hdr);
}
diff --git a/net/sctp/protocol.c b/net/sctp/protocol.c
index 989a900..a9e54ac 100644
--- a/net/sctp/protocol.c
+++ b/net/sctp/protocol.c
@@ -237,6 +237,38 @@ int sctp_copy_local_addr_list(struct net *net, struct sctp_bind_addr *bp,
return error;
}
+/* Copy over any ip options */
+static void sctp_v4_copy_ip_options(struct sock *sk, struct sock *newsk)
+{
+ struct inet_sock *newinet, *inet = inet_sk(sk);
+ struct ip_options_rcu *inet_opt, *newopt = NULL;
+
+ newinet = inet_sk(newsk);
+
+ rcu_read_lock();
+ inet_opt = rcu_dereference(inet->inet_opt);
+ if (inet_opt) {
+ newopt = sock_kmalloc(newsk, sizeof(*inet_opt) +
+ inet_opt->opt.optlen, GFP_ATOMIC);
+ if (newopt)
+ memcpy(newopt, inet_opt, sizeof(*inet_opt) +
+ inet_opt->opt.optlen);
+ }
+ RCU_INIT_POINTER(newinet->inet_opt, newopt);
+ rcu_read_unlock();
+}
+
+/* Account for the IP options */
+static int sctp_v4_ip_options_len(struct sock *sk)
+{
+ struct inet_sock *inet = inet_sk(sk);
+
+ if (inet->inet_opt)
+ return inet->inet_opt->opt.optlen;
+ else
+ return 0;
+}
+
/* Initialize a sctp_addr from in incoming skb. */
static void sctp_v4_from_skb(union sctp_addr *addr, struct sk_buff *skb,
int is_saddr)
@@ -590,6 +622,8 @@ static struct sock *sctp_v4_create_accept_sk(struct sock *sk,
sctp_copy_sock(newsk, sk, asoc);
sock_reset_flag(newsk, SOCK_ZAPPED);
+ sctp_v4_copy_ip_options(sk, newsk);
+
newinet = inet_sk(newsk);
newinet->inet_daddr = asoc->peer.primary_addr.v4.sin_addr.s_addr;
@@ -1008,6 +1042,7 @@ static struct sctp_pf sctp_pf_inet = {
.addr_to_user = sctp_v4_addr_to_user,
.to_sk_saddr = sctp_v4_to_sk_saddr,
.to_sk_daddr = sctp_v4_to_sk_daddr,
+ .copy_ip_options = sctp_v4_copy_ip_options,
.af = &sctp_af_inet
};
@@ -1092,6 +1127,7 @@ static struct sctp_af sctp_af_inet = {
.ecn_capable = sctp_v4_ecn_capable,
.net_header_len = sizeof(struct iphdr),
.sockaddr_len = sizeof(struct sockaddr_in),
+ .ip_options_len = sctp_v4_ip_options_len,
#ifdef CONFIG_COMPAT
.compat_setsockopt = compat_ip_setsockopt,
.compat_getsockopt = compat_ip_getsockopt,
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 8d76086..70355a0 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -3124,6 +3124,7 @@ static int sctp_setsockopt_maxseg(struct sock *sk, char __user *optval, unsigned
if (asoc) {
if (val == 0) {
val = asoc->pathmtu;
+ val -= sp->pf->af->ip_options_len(asoc->base.sk);
val -= sp->pf->af->net_header_len;
val -= sizeof(struct sctphdr) +
sizeof(struct sctp_data_chunk);
@@ -4917,9 +4918,11 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
sctp_copy_sock(sock->sk, sk, asoc);
/* Make peeled-off sockets more like 1-1 accepted sockets.
- * Set the daddr and initialize id to something more random
+ * Set the daddr and initialize id to something more random and also
+ * copy over any ip options.
*/
sp->pf->to_sk_daddr(&asoc->peer.primary_addr, sk);
+ sp->pf->copy_ip_options(sk, sock->sk);
/* Populate the fields of the newsk from the oldsk and migrate the
* asoc to the newsk.
--
2.13.6
^ permalink raw reply related
* [RFC PATCH 3/5] sctp: Add LSM hooks
From: Richard Haines @ 2017-10-17 13:58 UTC (permalink / raw)
To: selinux, netdev, linux-sctp, linux-security-module
Cc: paul, vyasevich, nhorman, sds, eparis, marcelo.leitner,
Richard Haines
Add security hooks to allow security modules to exercise access control
over SCTP.
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
include/net/sctp/structs.h | 10 ++++++++
include/uapi/linux/sctp.h | 1 +
net/sctp/sm_make_chunk.c | 12 +++++++++
net/sctp/sm_statefuns.c | 14 ++++++++++-
net/sctp/socket.c | 61 +++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 96 insertions(+), 2 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 7767577..6e72e3e 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -1270,6 +1270,16 @@ struct sctp_endpoint {
reconf_enable:1;
__u8 strreset_enable;
+
+ /* Security identifiers from incoming (INIT). These are set by
+ * security_sctp_assoc_request(). These will only be used by
+ * SCTP TCP type sockets and peeled off connections as they
+ * cause a new socket to be generated. security_sctp_sk_clone()
+ * will then plug these into the new socket.
+ */
+
+ u32 secid;
+ u32 peer_secid;
};
/* Recover the outter endpoint structure. */
diff --git a/include/uapi/linux/sctp.h b/include/uapi/linux/sctp.h
index 6217ff8..c04812f 100644
--- a/include/uapi/linux/sctp.h
+++ b/include/uapi/linux/sctp.h
@@ -122,6 +122,7 @@ typedef __s32 sctp_assoc_t;
#define SCTP_RESET_ASSOC 120
#define SCTP_ADD_STREAMS 121
#define SCTP_SOCKOPT_PEELOFF_FLAGS 122
+#define SCTP_SENDMSG_CONNECT 123
/* PR-SCTP policies */
#define SCTP_PR_SCTP_NONE 0x0000
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 6110447..ca4705b 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -3059,6 +3059,12 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
if (af->is_any(&addr))
memcpy(&addr, &asconf->source, sizeof(addr));
+ if (security_sctp_bind_connect(asoc->ep->base.sk,
+ SCTP_PARAM_ADD_IP,
+ (struct sockaddr *)&addr,
+ af->sockaddr_len))
+ return SCTP_ERROR_REQ_REFUSED;
+
/* ADDIP 4.3 D9) If an endpoint receives an ADD IP address
* request and does not have the local resources to add this
* new address to the association, it MUST return an Error
@@ -3125,6 +3131,12 @@ static __be16 sctp_process_asconf_param(struct sctp_association *asoc,
if (af->is_any(&addr))
memcpy(&addr.v4, sctp_source(asconf), sizeof(addr));
+ if (security_sctp_bind_connect(asoc->ep->base.sk,
+ SCTP_PARAM_SET_PRIMARY,
+ (struct sockaddr *)&addr,
+ af->sockaddr_len))
+ return SCTP_ERROR_REQ_REFUSED;
+
peer = sctp_assoc_lookup_paddr(asoc, &addr);
if (!peer)
return SCTP_ERROR_DNS_FAILED;
diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index b2a74c3..4ba5805 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -314,6 +314,11 @@ sctp_disposition_t sctp_sf_do_5_1B_init(struct net *net,
sctp_unrecognized_param_t *unk_param;
int len;
+ /* Update socket peer label if first association. */
+ if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
+ chunk->skb, SCTP_CID_INIT))
+ return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
/* 6.10 Bundling
* An endpoint MUST NOT bundle INIT, INIT ACK or
* SHUTDOWN COMPLETE with any other chunks.
@@ -446,7 +451,6 @@ sctp_disposition_t sctp_sf_do_5_1B_init(struct net *net,
}
sctp_add_cmd_sf(commands, SCTP_CMD_NEW_ASOC, SCTP_ASOC(new_asoc));
-
sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
/*
@@ -507,6 +511,11 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(struct net *net,
struct sctp_chunk *err_chunk;
struct sctp_packet *packet;
+ /* Update socket peer label if first association. */
+ if (security_sctp_assoc_request((struct sctp_endpoint *)ep,
+ chunk->skb, SCTP_CID_INIT_ACK))
+ return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
+
if (!sctp_vtag_verify(chunk, asoc))
return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands);
@@ -899,6 +908,9 @@ sctp_disposition_t sctp_sf_do_5_1E_ca(struct net *net,
*/
sctp_add_cmd_sf(commands, SCTP_CMD_INIT_COUNTER_RESET, SCTP_NULL());
+ /* Set peer label for connection. */
+ security_inet_conn_established(ep->base.sk, chunk->skb);
+
/* RFC 2960 5.1 Normal Establishment of an Association
*
* E) Upon reception of the COOKIE ACK, endpoint "A" will move
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 70355a0..e948163 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -1014,6 +1014,12 @@ static int sctp_setsockopt_bindx(struct sock *sk,
/* Do the work. */
switch (op) {
case SCTP_BINDX_ADD_ADDR:
+ /* Allow security module to validate bindx addresses. */
+ err = security_sctp_bind_connect(sk, SCTP_SOCKOPT_BINDX_ADD,
+ (struct sockaddr *)kaddrs,
+ addrs_size);
+ if (err)
+ goto out;
err = sctp_bindx_add(sk, kaddrs, addrcnt);
if (err)
goto out;
@@ -1223,6 +1229,7 @@ static int __sctp_connect(struct sock *sk,
if (assoc_id)
*assoc_id = asoc->assoc_id;
+
err = sctp_wait_for_connect(asoc, &timeo);
/* Note: the asoc may be freed after the return of
* sctp_wait_for_connect.
@@ -1336,9 +1343,17 @@ static int __sctp_setsockopt_connectx(struct sock *sk,
if (__copy_from_user(kaddrs, addrs, addrs_size)) {
err = -EFAULT;
} else {
+ /* Allow security module to validate connectx addresses. */
+ err = security_sctp_bind_connect(sk, SCTP_SOCKOPT_CONNECTX,
+ (struct sockaddr *)kaddrs,
+ addrs_size);
+ if (err)
+ goto out_free;
+
err = __sctp_connect(sk, kaddrs, addrs_size, assoc_id);
}
+out_free:
kfree(kaddrs);
return err;
@@ -1604,6 +1619,7 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
struct sctp_transport *transport, *chunk_tp;
struct sctp_chunk *chunk;
union sctp_addr to;
+ struct sctp_af *af;
struct sockaddr *msg_name = NULL;
struct sctp_sndrcvinfo default_sinfo;
struct sctp_sndrcvinfo *sinfo;
@@ -1833,6 +1849,24 @@ static int sctp_sendmsg(struct sock *sk, struct msghdr *msg, size_t msg_len)
}
scope = sctp_scope(&to);
+
+ /* Label connection socket for first association 1-to-many
+ * style for client sequence socket()->sendmsg(). This
+ * needs to be done before sctp_assoc_add_peer() as that will
+ * set up the initial packet that needs to account for any
+ * security ip options (CIPSO/CALIPSO) added to the packet.
+ */
+ af = sctp_get_af_specific(to.sa.sa_family);
+ if (!af) {
+ err = -EINVAL;
+ goto out_unlock;
+ }
+ err = security_sctp_bind_connect(sk, SCTP_SENDMSG_CONNECT,
+ (struct sockaddr *)&to,
+ af->sockaddr_len);
+ if (err < 0)
+ goto out_unlock;
+
new_asoc = sctp_association_new(ep, sk, scope, GFP_KERNEL);
if (!new_asoc) {
err = -ENOMEM;
@@ -2865,6 +2899,8 @@ static int sctp_setsockopt_primary_addr(struct sock *sk, char __user *optval,
{
struct sctp_prim prim;
struct sctp_transport *trans;
+ struct sctp_af *af;
+ int err;
if (optlen != sizeof(struct sctp_prim))
return -EINVAL;
@@ -2872,6 +2908,17 @@ static int sctp_setsockopt_primary_addr(struct sock *sk, char __user *optval,
if (copy_from_user(&prim, optval, sizeof(struct sctp_prim)))
return -EFAULT;
+ /* Allow security module to validate address but need address len. */
+ af = sctp_get_af_specific(prim.ssp_addr.ss_family);
+ if (!af)
+ return -EINVAL;
+
+ err = security_sctp_bind_connect(sk, SCTP_PRIMARY_ADDR,
+ (struct sockaddr *)&prim.ssp_addr,
+ af->sockaddr_len);
+ if (err)
+ return err;
+
trans = sctp_addr_id2transport(sk, &prim.ssp_addr, prim.ssp_assoc_id);
if (!trans)
return -EINVAL;
@@ -3192,6 +3239,13 @@ static int sctp_setsockopt_peer_primary_addr(struct sock *sk, char __user *optva
if (!sctp_assoc_lookup_laddr(asoc, (union sctp_addr *)&prim.sspp_addr))
return -EADDRNOTAVAIL;
+ /* Allow security module to validate address. */
+ err = security_sctp_bind_connect(sk, SCTP_SET_PEER_PRIMARY_ADDR,
+ (struct sockaddr *)&prim.sspp_addr,
+ af->sockaddr_len);
+ if (err)
+ return err;
+
/* Create an ASCONF chunk with SET_PRIMARY parameter */
chunk = sctp_make_asconf_set_prim(asoc,
(union sctp_addr *)&prim.sspp_addr);
@@ -8024,6 +8078,8 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
{
struct inet_sock *inet = inet_sk(sk);
struct inet_sock *newinet;
+ struct sctp_sock *sp = sctp_sk(sk);
+ struct sctp_endpoint *ep = sp->ep;
newsk->sk_type = sk->sk_type;
newsk->sk_bound_dev_if = sk->sk_bound_dev_if;
@@ -8066,7 +8122,10 @@ void sctp_copy_sock(struct sock *newsk, struct sock *sk,
if (newsk->sk_flags & SK_FLAGS_TIMESTAMP)
net_enable_timestamp();
- security_sk_clone(sk, newsk);
+ /* Set newsk security attributes from orginal sk and connection
+ * security attribute from ep.
+ */
+ security_sctp_sk_clone(ep, sk, newsk);
}
static inline void sctp_copy_descendant(struct sock *sk_to,
--
2.13.6
^ permalink raw reply related
* [RFC PATCH 1/5] security: Add support for SCTP security hooks
From: Richard Haines @ 2017-10-17 14:02 UTC (permalink / raw)
To: selinux, netdev, linux-sctp, linux-security-module
Cc: paul, vyasevich, nhorman, sds, eparis, marcelo.leitner,
Richard Haines
The SCTP security hooks are explained in:
Documentation/security/LSM-sctp.txt
Signed-off-by: Richard Haines <richard_c_haines@btinternet.com>
---
Documentation/security/LSM-sctp.txt | 212 ++++++++++++++++++++++++++++++++++++
include/linux/lsm_hooks.h | 37 +++++++
include/linux/security.h | 27 +++++
security/security.c | 23 ++++
4 files changed, 299 insertions(+)
create mode 100644 Documentation/security/LSM-sctp.txt
diff --git a/Documentation/security/LSM-sctp.txt b/Documentation/security/LSM-sctp.txt
new file mode 100644
index 0000000..30fe9b5
--- /dev/null
+++ b/Documentation/security/LSM-sctp.txt
@@ -0,0 +1,212 @@
+ SCTP LSM Support
+ ==================
+
+For security module support, three sctp specific hooks have been implemented:
+ security_sctp_assoc_request()
+ security_sctp_bind_connect()
+ security_sctp_sk_clone()
+
+Also the following security hook has been utilised:
+ security_inet_conn_established()
+
+The usage of these hooks are described below with the SELinux implementation
+described in Documentation/security/SELinux-sctp.txt
+
+
+security_sctp_assoc_request()
+------------------------------
+This new hook has been added to net/sctp/sm_statefuns.c where it passes the
+@ep and @chunk->skb (the association INIT or INIT ACK packet) to the security
+module. Returns 0 on success, error on failure.
+
+ @ep - pointer to sctp endpoint structure.
+ @skb - pointer to skbuff of association packet.
+ @sctp_cid - set to sctp packet type (SCTP_CID_INIT or SCTP_CID_INIT_ACK).
+
+The security module performs the following operations:
+ 1) If this is the first association on @ep->base.sk, then set the peer sid
+ to that in @skb. This will ensure there is only one peer sid assigned
+ to @ep->base.sk that may support multiple associations.
+
+ 2) If not the first association, validate the @ep->base.sk peer_sid against
+ the @skb peer sid to determine whether the association should be allowed
+ or denied.
+
+ 3) If @sctp_cid = SCTP_CID_INIT, then set the sctp @ep sid to socket's sid
+ (from ep->base.sk) with MLS portion taken from @skb peer sid. This will
+ only be used by SCTP TCP style sockets and peeled off connections as they
+ cause a new socket to be generated.
+
+ If IP security options are configured (CIPSO/CALIPSO), then the ip options
+ are set on the socket.
+
+ To support this hook include/net/sctp/structs.h "struct sctp_endpoint"
+ has been updated with the following:
+
+ /* Security identifiers from incoming (INIT). These are set by
+ * security_sctp_assoc_request(). These will only be used by
+ * SCTP TCP type sockets and peeled off connections as they
+ * cause a new socket to be generated. security_sctp_sk_clone()
+ * will then plug these into the new socket.
+ */
+ u32 secid;
+ u32 peer_secid;
+
+
+security_sctp_bind_connect()
+-----------------------------
+This new hook has been added to net/sctp/socket.c and net/sctp/sm_make_chunk.c.
+It passes one or more ipv4/ipv6 addresses to the security module for
+validation based on the @optname that will result in either a bind or connect
+service as shown in the permission check tables below.
+Returns 0 on success, error on failure.
+
+ @sk - Pointer to sock structure.
+ @optname - Name of the option to validate.
+ @address - One or more ipv4 / ipv6 addresses.
+ @addrlen - The total length of address(s). This is calculated on each
+ ipv4 or ipv6 address using sizeof(struct sockaddr_in) or
+ sizeof(struct sockaddr_in6).
+
+ ------------------------------------------------------------------
+ | BIND Type Checks |
+ | @optname | @address contains |
+ |----------------------------|-----------------------------------|
+ | SCTP_SOCKOPT_BINDX_ADD | One or more ipv4 / ipv6 addresses |
+ | SCTP_PRIMARY_ADDR | Single ipv4 or ipv6 address |
+ | SCTP_SET_PEER_PRIMARY_ADDR | Single ipv4 or ipv6 address |
+ ------------------------------------------------------------------
+
+ ------------------------------------------------------------------
+ | CONNECT Type Checks |
+ | @optname | @address contains |
+ |----------------------------|-----------------------------------|
+ | SCTP_SOCKOPT_CONNECTX | One or more ipv4 / ipv6 addresses |
+ | SCTP_PARAM_ADD_IP | One or more ipv4 / ipv6 addresses |
+ | SCTP_SENDMSG_CONNECT | Single ipv4 or ipv6 address |
+ | SCTP_PARAM_SET_PRIMARY | Single ipv4 or ipv6 address |
+ ------------------------------------------------------------------
+
+A summary of the @optname entries is as follows:
+
+ SCTP_SOCKOPT_BINDX_ADD - Allows additional bind addresses to be
+ associated after (optionally) calling
+ bind(3).
+ sctp_bindx(3) adds a set of bind
+ addresses on a socket.
+
+ SCTP_SOCKOPT_CONNECTX - Allows the allocation of multiple
+ addresses for reaching a peer
+ (multi-homed).
+ sctp_connectx(3) initiates a connection
+ on an SCTP socket using multiple
+ destination addresses.
+
+ SCTP_SENDMSG_CONNECT - Initiate a connection that is generated by a
+ sendmsg(2) or sctp_sendmsg(3) on a new asociation.
+
+ SCTP_PRIMARY_ADDR - Set local primary address.
+
+ SCTP_SET_PEER_PRIMARY_ADDR - Request peer sets address as
+ association primary.
+
+ SCTP_PARAM_ADD_IP - These are used when Dynamic Address
+ SCTP_PARAM_SET_PRIMARY - Reconfiguration is enabled as explained below.
+
+
+To support Dynamic Address Reconfiguration the following parameters must be
+enabled on both endpoints (or use the appropriate setsockopts):
+ /proc/sys/net/sctp/addip_enable
+ /proc/sys/net/sctp/addip_noauth_enable
+
+then the following *_PARAM_*'s are sent to the peer in an
+ASCONF chunk when the corresponding @optname's are present:
+
+ @optname ASCONF Parameter
+ SCTP_SOCKOPT_BINDX_ADD -> SCTP_PARAM_ADD_IP
+ SCTP_SET_PEER_PRIMARY_ADDR -> SCTP_PARAM_SET_PRIMARY
+
+
+security_sctp_sk_clone()
+-------------------------
+This new hook has been added to net/sctp/socket.c sctp_sock_migrate() that is
+called whenever a new socket is created by accept(2) (i.e. a TCP style socket)
+or when a socket is 'peeled off' e.g userspace calls sctp_peeloff(3).
+security_sctp_sk_clone() will set the new sockets sid and peer sid to that
+contained in the @ep sid and @ep peer sid respectively.
+
+ @ep - pointer to old sctp endpoint structure.
+ @sk - pointer to old sock structure.
+ @sk - pointer to new sock structure.
+
+security_inet_conn_established()
+---------------------------------
+This hook has been added to net/sctp/sm_statefuns.c COOKIE ECHO processing
+where it sets the connection's peer sid to that in @skb.
+
+ @sk - pointer to sock structure.
+ @skb - pointer to skbuff of the COOKIE ECHO packet.
+
+
+Security Hooks used for Association Establishment
+==================================================
+The following diagram shows the use of security_sctp_connect_bind(),
+security_sctp_assoc_request(), security_inet_conn_established() in
+net/sctp/sm_statefuns.c and security_sctp_sk_clone() in net/sctp/socket.c,
+when establishing an association.
+
+ SCTP endpoint "A" SCTP endpoint "Z"
+ ================= =================
+ sctp_sf_do_prm_asoc()
+ Association setup can be initiated
+ by a connect(2), sctp_connectx(3),
+ sendmsg(2) or sctp_sendmsg(3).
+ These will result in a call to
+ security_sctp_bind_connect() to
+ initiate an association to
+ SCTP peer endpoint "Z".
+ INIT --------------------------------------------->
+ sctp_sf_do_5_1B_init()
+ Respond to an INIT chunk.
+ SCTP peer endpoint "A" is
+ asking for an association. Call
+ security_sctp_assoc_request()
+ to set the peer label if first
+ association.
+ If not first association, check
+ whether allowed, IF so send:
+ <----------------------------------------------- INIT ACK
+ | ELSE audit event and silently
+ | discard the packet.
+ sctp_sf_do_5_1C_ack
+ Respond to an INIT ACK chunk.
+ SCTP peer endpoint"A" initiated
+ this association to SCTP peer
+ endpoint "Z". Call
+ security_sctp_assoc_request()
+ to set the peer label if first
+ association. If not first
+ association, check whether
+ allowed, IF so send:
+ COOKIE ECHO ------------------------------------------>
+ ELSE audit event and silently |
+ discard the packet. |
+ |
+ <------------------------------------------- COOKIE ACK
+ | |
+ sctp_sf_do_5_1E_ca |
+ Call security_inet_conn_established() |
+ to set the correct peer sid. |
+ | |
+ | net/sctp/socket.c sctp_copy_sock()
+ | If SCTP_SOCKET_TCP or peeled off
+ | socket security_sctp_sk_clone() is
+ | called to clone the new socket.
+ | |
+ ESTABLISHED ESTABLISHED
+ | |
+ ------------------------------------------------------------------
+ | Association Established |
+ ------------------------------------------------------------------
+
+
diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
index 3a90feb..42370a7 100644
--- a/include/linux/lsm_hooks.h
+++ b/include/linux/lsm_hooks.h
@@ -913,6 +913,33 @@
* associated with the TUN device's security structure.
* @security pointer to the TUN devices's security structure.
*
+ * Security hooks for SCTP
+ *
+ * @sctp_assoc_request:
+ * If first association, then set the peer sid to that in @skb. If
+ * @sctp_cid is from an INIT chunk, then set the sctp endpoint sid to
+ * socket's sid (ep->base.sk) with MLS portion taken from peer sid.
+ * @ep pointer to sctp endpoint structure.
+ * @skb pointer to skbuff of association packet.
+ * @sctp_cid whether association from INIT or INIT_ACK chunk.
+ * Return 0 on success, error on failure.
+ * @sctp_bind_connect:
+ * Validiate permissions required for each address associated with sock
+ * @sk. Depending on @optname, the addresses will be treated as either
+ * for a connect or bind service. The @addrlen is calculated on each
+ * ipv4 and ipv6 address using sizeof(struct sockaddr_in) or
+ * sizeof(struct sockaddr_in6).
+ * @sk pointer to sock structure.
+ * @optname name of the option to validate.
+ * @address list containing one or more ipv4/ipv6 addresses.
+ * @addrlen total length of address(s).
+ * Return 0 on success, error on failure.
+ * @sctp_sk_clone:
+ * Sets the new child socket's sid to the old endpoint sid.
+ * @ep pointer to old sctp endpoint structure.
+ * @sk pointer to old sock structure.
+ * @sk pointer to new sock structure.
+ *
* Security hooks for Infiniband
*
* @ib_pkey_access:
@@ -1640,6 +1667,13 @@ union security_list_options {
int (*tun_dev_attach_queue)(void *security);
int (*tun_dev_attach)(struct sock *sk, void *security);
int (*tun_dev_open)(void *security);
+ int (*sctp_assoc_request)(struct sctp_endpoint *ep,
+ struct sk_buff *skb,
+ int sctp_cid);
+ int (*sctp_bind_connect)(struct sock *sk, int optname,
+ struct sockaddr *address, int addrlen);
+ void (*sctp_sk_clone)(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk);
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_INFINIBAND
@@ -1880,6 +1914,9 @@ struct security_hook_heads {
struct list_head tun_dev_attach_queue;
struct list_head tun_dev_attach;
struct list_head tun_dev_open;
+ struct list_head sctp_assoc_request;
+ struct list_head sctp_bind_connect;
+ struct list_head sctp_sk_clone;
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_INFINIBAND
struct list_head ib_pkey_access;
diff --git a/include/linux/security.h b/include/linux/security.h
index 834b355..2054023 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -114,6 +114,7 @@ struct xfrm_policy;
struct xfrm_state;
struct xfrm_user_sec_ctx;
struct seq_file;
+struct sctp_endpoint;
#ifdef CONFIG_MMU
extern unsigned long mmap_min_addr;
@@ -1240,6 +1241,12 @@ int security_tun_dev_create(void);
int security_tun_dev_attach_queue(void *security);
int security_tun_dev_attach(struct sock *sk, void *security);
int security_tun_dev_open(void *security);
+int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb,
+ int sctp_cid);
+int security_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address, int addrlen);
+void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk);
#else /* CONFIG_SECURITY_NETWORK */
static inline int security_unix_stream_connect(struct sock *sock,
@@ -1432,6 +1439,26 @@ static inline int security_tun_dev_open(void *security)
{
return 0;
}
+
+static inline int security_sctp_assoc_request(struct sctp_endpoint *ep,
+ struct sk_buff *skb,
+ int sctp_cid)
+{
+ return 0;
+}
+
+static inline int security_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address,
+ int addrlen)
+{
+ return 0;
+}
+
+static inline void security_sctp_sk_clone(struct sctp_endpoint *ep,
+ struct sock *sk,
+ struct sock *newsk)
+{
+}
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_INFINIBAND
diff --git a/security/security.c b/security/security.c
index 3013237..798fc6e 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1482,6 +1482,7 @@ void security_inet_conn_established(struct sock *sk,
{
call_void_hook(inet_conn_established, sk, skb);
}
+EXPORT_SYMBOL(security_inet_conn_established);
int security_secmark_relabel_packet(u32 secid)
{
@@ -1537,6 +1538,28 @@ int security_tun_dev_open(void *security)
}
EXPORT_SYMBOL(security_tun_dev_open);
+int security_sctp_assoc_request(struct sctp_endpoint *ep, struct sk_buff *skb,
+ int sctp_cid)
+{
+ return call_int_hook(sctp_assoc_request, 0, ep, skb, sctp_cid);
+}
+EXPORT_SYMBOL(security_sctp_assoc_request);
+
+int security_sctp_bind_connect(struct sock *sk, int optname,
+ struct sockaddr *address, int addrlen)
+{
+ return call_int_hook(sctp_bind_connect, 0, sk, optname,
+ address, addrlen);
+}
+EXPORT_SYMBOL(security_sctp_bind_connect);
+
+void security_sctp_sk_clone(struct sctp_endpoint *ep, struct sock *sk,
+ struct sock *newsk)
+{
+ call_void_hook(sctp_sk_clone, ep, sk, newsk);
+}
+EXPORT_SYMBOL(security_sctp_sk_clone);
+
#endif /* CONFIG_SECURITY_NETWORK */
#ifdef CONFIG_SECURITY_INFINIBAND
--
2.13.6
^ permalink raw reply related
* Re: [PATCH v9 00/20] simplify crypto wait for async op
From: Russell King - ARM Linux @ 2017-10-17 14:06 UTC (permalink / raw)
To: Gilad Ben-Yossef
Cc: Herbert Xu, David S. Miller, Jonathan Corbet, David Howells,
Tom Lendacky, Gary Hook, Boris Brezillon, Arnaud Ebalard,
Matthias Brugger, Alasdair Kergon, Mike Snitzer, dm-devel,
Steve French, Theodore Y. Ts'o, Jaegeuk Kim, Steffen Klassert,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Mimi Zohar,
Dmitry Kasatkin <dmitry.ka
In-Reply-To: <1508059209-25529-1-git-send-email-gilad@benyossef.com>
On Sun, Oct 15, 2017 at 10:19:45AM +0100, Gilad Ben-Yossef wrote:
> Many users of kernel async. crypto services have a pattern of
> starting an async. crypto op and than using a completion
> to wait for it to end.
>
> This patch set simplifies this common use case in two ways:
>
> First, by separating the return codes of the case where a
> request is queued to a backlog due to the provider being
> busy (-EBUSY) from the case the request has failed due
> to the provider being busy and backlogging is not enabled
> (-EAGAIN).
>
> Next, this change is than built on to create a generic API
> to wait for a async. crypto operation to complete.
>
> The end result is a smaller code base and an API that is
> easier to use and more difficult to get wrong.
>
> The patch set was boot tested on x86_64 and arm64 which
> at the very least tests the crypto users via testmgr and
> tcrypt but I do note that I do not have access to some
> of the HW whose drivers are modified nor do I claim I was
> able to test all of the corner cases.
>
> The patch set is based upon linux-next release tagged
> next-20171013.
Has there been any performance impact analysis of these changes? I
ended up with patches for one of the crypto drivers which converted
its interrupt handling to threaded interrupts being reverted because
it caused a performance degredation.
Moving code to latest APIs to simplify it is not always beneficial.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: Linux 4.12+ memory leak on router with i40e NICs
From: Paweł Staszewski @ 2017-10-17 14:08 UTC (permalink / raw)
To: Alexander Duyck
Cc: Pavlos Parissis, Anders K. Pedersen | Cohaesio,
netdev@vger.kernel.org, intel-wired-lan@lists.osuosl.org,
alexander.h.duyck@intel.com
In-Reply-To: <efcc51b4-f911-11ec-1c56-fcf31081f9d5@itcare.pl>
W dniu 2017-10-17 o 13:52, Paweł Staszewski pisze:
>
>
> W dniu 2017-10-17 o 13:05, Paweł Staszewski pisze:
>>
>>
>> W dniu 2017-10-17 o 12:59, Paweł Staszewski pisze:
>>>
>>>
>>> W dniu 2017-10-17 o 12:51, Paweł Staszewski pisze:
>>>>
>>>>
>>>> W dniu 2017-10-17 o 12:20, Paweł Staszewski pisze:
>>>>>
>>>>>
>>>>> W dniu 2017-10-17 o 11:48, Paweł Staszewski pisze:
>>>>>>
>>>>>>
>>>>>> W dniu 2017-10-17 o 02:44, Paweł Staszewski pisze:
>>>>>>>
>>>>>>>
>>>>>>> W dniu 2017-10-17 o 01:56, Alexander Duyck pisze:
>>>>>>>> On Mon, Oct 16, 2017 at 4:34 PM, Paweł Staszewski
>>>>>>>> <pstaszewski@itcare.pl> wrote:
>>>>>>>>>
>>>>>>>>> W dniu 2017-10-16 o 18:26, Paweł Staszewski pisze:
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> W dniu 2017-10-16 o 13:20, Pavlos Parissis pisze:
>>>>>>>>>>> On 15/10/2017 02:58 πμ, Alexander Duyck wrote:
>>>>>>>>>>>> Hi Pawel,
>>>>>>>>>>>>
>>>>>>>>>>>> To clarify is that Dave Miller's tree or Linus's that you
>>>>>>>>>>>> are talking
>>>>>>>>>>>> about? If it is Dave's tree how long ago was it you pulled
>>>>>>>>>>>> it since I
>>>>>>>>>>>> think the fix was just pushed by Jeff Kirsher a few days ago.
>>>>>>>>>>>>
>>>>>>>>>>>> The issue should be fixed in the following commit:
>>>>>>>>>>>>
>>>>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/commit/drivers/net/ethernet/intel/i40e/i40e_txrx.c?id=2b9478ffc550f17c6cd8c69057234e91150f5972
>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>> Do you know when it is going to be available on net-next and
>>>>>>>>>>> linux-stable
>>>>>>>>>>> repos?
>>>>>>>>>>>
>>>>>>>>>>> Cheers,
>>>>>>>>>>> Pavlos
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>> I will make some tests today night with "net" git tree where
>>>>>>>>>> this patch is
>>>>>>>>>> included.
>>>>>>>>>> Starting from 0:00 CET
>>>>>>>>>> :)
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>> Upgraded and looks like problem is not solved with that patch
>>>>>>>>> Currently running system with
>>>>>>>>> https://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git/
>>>>>>>>> kernel
>>>>>>>>>
>>>>>>>>> Still about 0.5GB of memory is leaking somewhere
>>>>>>>>>
>>>>>>>>> Also can confirm that the latest kernel where memory is not
>>>>>>>>> leaking (with
>>>>>>>>> use i40e driver intel 710 cards) is 4.11.12
>>>>>>>>> With kernel 4.11.12 - after hour no change in memory usage.
>>>>>>>>>
>>>>>>>>> also checked that with ixgbe instead of i40e with same net.git
>>>>>>>>> kernel there
>>>>>>>>> is no memleak - after hour same memory usage - so for 100%
>>>>>>>>> this is i40e
>>>>>>>>> driver problem.
>>>>>>>> So how long was the run to get the .5GB of memory leaking?
>>>>>>> 1 hour
>>>>>>>
>>>>>>>>
>>>>>>>> Also is there any chance of you being able to bisect to determine
>>>>>>>> where the memory leak was introduced since as you pointed out it
>>>>>>>> didn't exist in 4.11.12 so odds are it was introduced somewhere
>>>>>>>> between 4.11 and the latest kernel release.
>>>>>>> Can be hard cause currently need to back to 4.11.12 - this is
>>>>>>> production host/router
>>>>>>> Will try to find some free/test router for tests/bicects with
>>>>>>> i40e driver (intel 710 cards)
>>>>>>>
>>>>>>>>
>>>>>>>> Thanks.
>>>>>>>>
>>>>>>>> - Alex
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> Also forgoto to add errors for i40e when driver initialize:
>>>>>> [ 15.760569] i40e 0000:02:00.1: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.365587] i40e 0000:03:00.3: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.367686] i40e 0000:02:00.2: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.368816] i40e 0000:03:00.0: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.369877] i40e 0000:03:00.2: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.370941] i40e 0000:02:00.3: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.372005] i40e 0000:02:00.0: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>> [ 16.373029] i40e 0000:03:00.1: Error I40E_AQ_RC_ENOSPC adding
>>>>>> RX filters on PF, promiscuous mode forced on
>>>>>>
>>>>>> some params that are set for this nic's
>>>>>> ip link set up dev $i
>>>>>> ethtool -A $i autoneg off rx off tx off
>>>>>> ethtool -G $i rx 1024 tx 2048
>>>>>> ip link set $i txqueuelen 1000
>>>>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs
>>>>>> 512 tx-usecs 128
>>>>>> ethtool -L $i combined 6
>>>>>> #ethtool -N $i rx-flow-hash udp4 sdfn
>>>>>> ethtool -K $i ntuple on
>>>>>> ethtool -K $i gro off
>>>>>> ethtool -K $i tso off
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>> Also after TSO/GRO on there is memory usage change - and leaking
>>>>> faster
>>>>> Below image from memory usage before change with TSO/GRO OFF and
>>>>> after enabling TSO/GRO
>>>>>
>>>>> https://ibb.co/dTqBY6
>>>>>
>>>>>
>>>>> Thanks
>>>>> Pawel
>>>>>
>>>>>
>>>>>
>>>> With settings like this:
>>>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>>>> enp3s0f3'
>>>> for i in $ifc
>>>> do
>>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 512
>>>> tx-usecs 128
>>>> ethtool -K $i gro on
>>>> ethtool -K $i tso on
>>>>
>>>> done
>>>>
>>>> Server is leaking about 4-6MB per each 10 seconds
>>>> MEMLEAK:
>>>> 5 MB/10sec
>>>> 6 MB/10sec
>>>> 4 MB/10sec
>>>> 4 MB/10sec
>>>>
>>>>
>>>> Other settings TSO/GRO off
>>>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>>>> enp3s0f3'
>>>> for i in $ifc
>>>> do
>>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 512
>>>> tx-usecs 128
>>>> ethtool -K $i gro off
>>>> ethtool -K $i tso off
>>>>
>>>> done
>>>>
>>>> Same leak about 5MB per 10 seconds
>>>> MEMLEAK:
>>>> 5 MB/10sec
>>>> 5 MB/10sec
>>>> 5 MB/10sec
>>>>
>>>>
>>>> Other settings rx-usecs change from 512 to 1024:
>>>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>>>> enp3s0f3'
>>>> for i in $ifc
>>>> do
>>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 1024
>>>> tx-usecs 128
>>>> ethtool -K $i gro off
>>>> ethtool -K $i tso off
>>>>
>>>> done
>>>>
>>>> MEMLEAK:
>>>> 4 MB/10sec
>>>> 3 MB/10sec
>>>> 4 MB/10sec
>>>> 4 MB/10sec
>>>>
>>>>
>>>> So memleak have something to do with rx-usecs (less interrupts but
>>>> bigger latency for traffic)
>>>>
>>>>
>>>> But also enabling TSO/GRO making leak about 1MB bigger for each 10
>>>> seconds
>>>>
>>>>
>>>>
>>> So far best config is:
>>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>>> enp3s0f3'
>>> for i in $ifc
>>> do
>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 64
>>> tx-usecs 512
>>> ethtool -K $i gro off
>>> ethtool -K $i tso on
>>>
>>> done
>>>
>>> MEMLEAK - about 2MB/10secs
>>> 2 MB/10sec
>>> 2 MB/10sec
>>> 2 MB/10sec
>>>
>>>
>>> With - rx-usecs set to 256 (about 7-9MB/10secs memleak)
>>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>>> enp3s0f3'
>>> for i in $ifc
>>> do
>>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 256
>>> tx-usecs 512
>>> ethtool -K $i gro off
>>> ethtool -K $i tso on
>>>
>>> done
>>>
>>> MEMLEAK:
>>> 7 MB/10sec
>>> 7 MB/10sec
>>> 8 MB/10sec
>>> 9 MB/10sec
>>>
>>>
>>
>> And even less memleak with rx-usecs set to 32
>> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
>> enp3s0f3'
>> for i in $ifc
>> do
>> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 32
>> tx-usecs 512
>> ethtool -K $i gro off
>> ethtool -K $i tso on
>>
>> done
>>
>>
>> MEMLEAK - about 0-2MB for each 10 seconds
>> 0 MB/10sec
>> 1 MB/10sec
>> 0 MB/10sec
>> 2 MB/10sec
>> 1 MB/10sec
>>
>>
>>
>
>
> So best settings - to have as less leak as possible for now (rx-usecs
> set to 16):
> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
> enp3s0f3'
> for i in $ifc
> do
> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 16
> tx-usecs 768
> ethtool -K $i gro on
> ethtool -K $i tso on
>
> done
>
>
> MEMLEAK: (0-1MB/10seconds)
> 0 MB/10sec
> 0 MB/10sec
> 0 MB/10sec
> 1 MB/10sec
> 1 MB/10sec
> -1 MB/10sec
> 1 MB/10sec
> 1 MB/10sec
> 0 MB/10sec
>
> (there are some memory recycles - so this is good :) )
>
>
>
> Compared to(rx-usecs 512):
>
> ifc='enp2s0f0 enp2s0f1 enp2s0f2 enp2s0f3 enp3s0f0 enp3s0f1 enp3s0f2
> enp3s0f3'
> for i in $ifc
> do
> ethtool -C $i adaptive-rx off adaptive-tx off rx-usecs 512
> tx-usecs 128
> ethtool -K $i gro on
> ethtool -K $i tso on
>
> done
>
> Server is leaking about 4-6MB per each 10 seconds
> MEMLEAK:
> 5 MB/10sec
> 6 MB/10sec
> 4 MB/10sec
> 4 MB/10sec
>
>
And graph where all changes for rx-usecs was done over some time:
https://ibb.co/nrRfbR
^ permalink raw reply
* Re: [PATCH 00/58] networking: Convert timers to use timer_setup()
From: Kalle Valo @ 2017-10-17 14:18 UTC (permalink / raw)
To: Kees Cook
Cc: David S. Miller, netdev, Thomas Gleixner, linux-kernel,
linux-wireless
In-Reply-To: <1508200182-104605-1-git-send-email-keescook@chromium.org>
+ linux-wireless
Hi Kees,
Kees Cook <keescook@chromium.org> writes:
> This is the current set of outstanding networking patches to perform
> conversions to the new timer interface (rebased to -next). This is not
> all expected conversions, but it contains everything needed in networking
> to eliminate init_timer(), and all the non-standard setup_*_timer() uses.
So this also includes patches which I had queued for
wireless-drivers-next:
https://patchwork.kernel.org/patch/9986253/
https://patchwork.kernel.org/patch/9986245/
And looking at patchwork[1] I have even more timer_setup() related
patches from you. It would be really helpful if you could clearly
document to which tree you want the patches to be applied. I don't care
if it's net-next or wireless-drivers-next as long as it's not the both
(meaning that both Dave and me apply the same patch, which would be
bad). The thing is that I really do not have time to figure out for
every patch via which tree it's supposed to go.
For now I'll just drop all your timer_setup() related patches from my
queue and I'll assume Dave will take those. Ok?
[1] https://patchwork.kernel.org/project/linux-wireless/list/
--
Kalle Valo
^ permalink raw reply
* RE: [PATCH] net: export netdev_txq_to_tc to allow sch_mqprio to compile as module
From: Duyck, Alexander H @ 2017-10-17 14:31 UTC (permalink / raw)
To: Eric Dumazet, Henrik Austad
Cc: netdev, David S . Miller, Daniel Borkmann, David Ahern,
Willem de Bruijn, John Fastabend, tcharding, LKML, Henrik Austad,
Sanchez-Palencia, Jesus
In-Reply-To: <CANn89iJPRjokxHmj4dYnBQkJ=A4Z-7cMpZ34L+i949tJwUXd2g@mail.gmail.com>
> -----Original Message-----
> From: Eric Dumazet [mailto:edumazet@google.com]
> Sent: Tuesday, October 17, 2017 4:21 AM
> To: Henrik Austad <henrik@austad.us>
> Cc: netdev <netdev@vger.kernel.org>; David S . Miller
> <davem@davemloft.net>; Daniel Borkmann <daniel@iogearbox.net>; David
> Ahern <dsahern@gmail.com>; Duyck, Alexander H
> <alexander.h.duyck@intel.com>; Willem de Bruijn <willemb@google.com>;
> John Fastabend <john.fastabend@gmail.com>; tcharding <me@tobin.cc>; LKML
> <linux-kernel@vger.kernel.org>; Henrik Austad <haustad@cisco.com>; Sanchez-
> Palencia, Jesus <jesus.sanchez-palencia@intel.com>
> Subject: Re: [PATCH] net: export netdev_txq_to_tc to allow sch_mqprio to
> compile as module
>
> On Tue, Oct 17, 2017 at 3:10 AM, Henrik Austad <henrik@austad.us> wrote:
> > In commit 32302902ff09 ("mqprio: Reserve last 32 classid values for HW
> > traffic classes and misc IDs") sch_mqprio started using
> > netdev_txq_to_tc to find the correct tc instead of dev->tc_to_txq[]
> >
> > However, when mqprio is compiled as a module, it cannot resolve the
> > symbol, leading to this error:
> >
> > ERROR: "netdev_txq_to_tc" [net/sched/sch_mqprio.ko] undefined!
> >
> > This adds an EXPORT_SYMBOL() since the other user in the kernel
> > (netif_set_xps_queue) is also EXPORT_SYMBOL() (and not _GPL) or in a
> > sysfs-callback.
> >
> > Cc: Alexander Duyck <alexander.h.duyck@intel.com>
> > Cc: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
> > Cc: David S. Miller <davem@davemloft.net>
> > Signed-off-by: Henrik Austad <haustad@cisco.com>
>
>
> Reviewed-by: Eric Dumazet <edumazet@google.com>
This is identical to a patch I submitted yesterday when I got the report from the kbuild robot. I would say your description looks much better than mine though so I would be good with dropping my patch in favor of this one.
Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
^ permalink raw reply
* [PATCH] vmxnet3: Use correct minimum MTU value
From: Mohammed Gamal @ 2017-10-17 14:33 UTC (permalink / raw)
To: netdev, skhare, pv-drivers
Cc: linux-kernel, cavery, otubo, vkuznets, Mohammed Gamal
Currently the vmxnet3 driver has a minimum MTU value of 60. Which
goes against the RFC791 spec which specifies it at 68.
Setting MTU to values between 60 <= MTU <= 67 causes the network
interface to lose its IP, and it fails to restart.
This sets the minimum value to ETH_MIN_MTU (68) which is compatible
with is according to spec.
Reported-by: Bo Yang <boyang@redhat.com>
Signed-off-by: Mohammed Gamal <mgamal@redhat.com>
---
drivers/net/vmxnet3/vmxnet3_defs.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/vmxnet3/vmxnet3_defs.h b/drivers/net/vmxnet3/vmxnet3_defs.h
index c3a3164..4ad905a 100644
--- a/drivers/net/vmxnet3/vmxnet3_defs.h
+++ b/drivers/net/vmxnet3/vmxnet3_defs.h
@@ -749,7 +749,7 @@ struct Vmxnet3_DriverShared {
((vfTable[vid >> 5] & (1 << (vid & 31))) != 0)
#define VMXNET3_MAX_MTU 9000
-#define VMXNET3_MIN_MTU 60
+#define VMXNET3_MIN_MTU ETH_MIN_MTU
#define VMXNET3_LINK_UP (10000 << 16 | 1) /* 10 Gbps, up */
#define VMXNET3_LINK_DOWN 0
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net-next 27/34] nfp: bpf: Convert ndo_setup_tc offloads to block callbacks
From: Jakub Kicinski @ 2017-10-17 14:39 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, Daniel Borkmann
In-Reply-To: <20171017124812.GH2112@nanopsycho>
On Tue, 17 Oct 2017 14:48:12 +0200, Jiri Pirko wrote:
> Fri, Oct 13, 2017 at 03:08:24AM CEST, jakub.kicinski@netronome.com wrote:
> >On Thu, 12 Oct 2017 19:18:16 +0200, Jiri Pirko wrote:
> >> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/offload.c b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
> >> index a88bb5b..9e9af88 100644
> >> --- a/drivers/net/ethernet/netronome/nfp/bpf/offload.c
> >> +++ b/drivers/net/ethernet/netronome/nfp/bpf/offload.c
> >> @@ -246,6 +246,10 @@ int nfp_net_bpf_offload(struct nfp_net *nn, struct tc_cls_bpf_offload *cls_bpf)
> >> void *code;
> >> int err;
> >>
> >> + if (cls_bpf->common.protocol != htons(ETH_P_ALL) ||
> >> + cls_bpf->common.chain_index)
> >> + return -EOPNOTSUPP;
> >> +
> >> max_instr = nn_readw(nn, NFP_NET_CFG_BPF_MAX_LEN);
> >>
> >> switch (cls_bpf->command) {
> >
> >It is certainly very ugly but I send a fake struct tc_cls_bpf_offload
> >here for XDP. Refactoring this mess is pretty high on my priority list
> >but one way or the other this function will be called from XDP so TC
> >checks must stay in the TC handler... :(
>
> Okay. But currently, why is it a problem? You don't need the checks for
> xdp path.
>
static int
nfp_bpf_xdp_offload(struct nfp_app *app, struct nfp_net *nn,
struct bpf_prog *prog)
{
struct tc_cls_bpf_offload cmd = {
.prog = prog,
};
int ret;
if (!nfp_net_ebpf_capable(nn))
return -EINVAL;
if (nn->dp.ctrl & NFP_NET_CFG_CTRL_BPF) {
if (!nn->dp.bpf_offload_xdp)
return prog ? -EBUSY : 0;
cmd.command = prog ? TC_CLSBPF_REPLACE : TC_CLSBPF_DESTROY;
} else {
if (!prog)
return 0;
cmd.command = TC_CLSBPF_ADD;
}
ret = nfp_net_bpf_offload(nn, &cmd);
/* Stop offload if replace not possible */
if (ret && cmd.command == TC_CLSBPF_REPLACE)
nfp_bpf_xdp_offload(app, nn, NULL);
nn->dp.bpf_offload_xdp = prog && !ret;
return ret;
}
The fake offload struct is at the top of this function. Dereferencing
cls_bpf->common in nfp_net_bpf_offload() will crash the kernel. Or am
I missing something?
^ permalink raw reply
* [RFC] Can I use MSG_WAITALL with sendmsg() for AF_RXRPC?
From: David Howells @ 2017-10-17 14:55 UTC (permalink / raw)
To: netdev; +Cc: dhowells
Can I MSG_WAITALL with sendmsg() when sending data through an AF_RXRPC socket
to say:
Ignore signals until all the given data is queued, provided that
progress is reasonably made.
where "progress" is defined as:
Within a 2*RTT timeout since the last time we checked, at least one
DATA packet has been consumed on the other side.
Note that consumption by the other side doesn't guarantee space in the Tx
queue on this side if the other side shrinks its Rx window on us.
Or should I use a sockopt or sendmsg cmsg for this?
(Example change attached)
Thanks,
David
---
diff --git a/net/rxrpc/sendmsg.c b/net/rxrpc/sendmsg.c
index 9ea6f972767e..2d9edc656ca3 100644
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -38,12 +38,86 @@ struct rxrpc_send_params {
};
/*
+ * Wait for space to appear in the Tx queue or a signal to occur.
+ */
+static int rxrpc_wait_for_tx_window_intr(struct rxrpc_sock *rx,
+ struct rxrpc_call *call,
+ long *timeo)
+{
+ for (;;) {
+ set_current_state(TASK_INTERRUPTIBLE);
+ if (call->tx_top - call->tx_hard_ack <
+ min_t(unsigned int, call->tx_winsize,
+ call->cong_cwnd + call->cong_extra))
+ return 0;
+
+ if (call->state >= RXRPC_CALL_COMPLETE)
+ return call->error;
+
+ if (signal_pending(current))
+ return sock_intr_errno(*timeo);
+
+ trace_rxrpc_transmit(call, rxrpc_transmit_wait);
+ mutex_unlock(&call->user_mutex);
+ *timeo = schedule_timeout(*timeo);
+ if (mutex_lock_interruptible(&call->user_mutex) < 0)
+ return sock_intr_errno(*timeo);
+ }
+}
+
+/*
+ * Wait for space to appear in the Tx queue uninterruptibly, but with
+ * a timeout of 2*RTT if no progress was made and a signal occurred.
+ */
+static int rxrpc_wait_for_tx_window_nonintr(struct rxrpc_sock *rx,
+ struct rxrpc_call *call)
+{
+ rxrpc_seq_t tx_start, tx_win;
+ signed long rtt2, timeout;
+ u64 rtt;
+
+ rtt = READ_ONCE(call->peer->rtt);
+ rtt2 = nsecs_to_jiffies64(rtt) * 2;
+ if (rtt2 < 1)
+ rtt2 = 1;
+
+ timeout = rtt2;
+ tx_start = READ_ONCE(call->tx_hard_ack);
+
+ for (;;) {
+ set_current_state(TASK_UNINTERRUPTIBLE);
+
+ tx_win = READ_ONCE(call->tx_hard_ack);
+ if (call->tx_top - tx_win <
+ min_t(unsigned int, call->tx_winsize,
+ call->cong_cwnd + call->cong_extra))
+ return 0;
+
+ if (call->state >= RXRPC_CALL_COMPLETE)
+ return call->error;
+
+ if (timeout == 0 &&
+ tx_win == tx_start && signal_pending(current))
+ return -EINTR;
+
+ if (tx_win != tx_start) {
+ timeout = rtt2;
+ tx_start = tx_win;
+ }
+
+ trace_rxrpc_transmit(call, rxrpc_transmit_wait);
+ timeout = schedule_timeout(timeout);
+ }
+}
+
+/*
* wait for space to appear in the transmit/ACK window
* - caller holds the socket locked
*/
static int rxrpc_wait_for_tx_window(struct rxrpc_sock *rx,
struct rxrpc_call *call,
- long *timeo)
+ long *timeo,
+ bool waitall)
{
DECLARE_WAITQUEUE(myself, current);
int ret;
@@ -53,30 +127,10 @@ static int rxrpc_wait_for_tx_window(struct rxrpc_sock *rx,
add_wait_queue(&call->waitq, &myself);
- for (;;) {
- set_current_state(TASK_INTERRUPTIBLE);
- ret = 0;
- if (call->tx_top - call->tx_hard_ack <
- min_t(unsigned int, call->tx_winsize,
- call->cong_cwnd + call->cong_extra))
- break;
- if (call->state >= RXRPC_CALL_COMPLETE) {
- ret = call->error;
- break;
- }
- if (signal_pending(current)) {
- ret = sock_intr_errno(*timeo);
- break;
- }
-
- trace_rxrpc_transmit(call, rxrpc_transmit_wait);
- mutex_unlock(&call->user_mutex);
- *timeo = schedule_timeout(*timeo);
- if (mutex_lock_interruptible(&call->user_mutex) < 0) {
- ret = sock_intr_errno(*timeo);
- break;
- }
- }
+ if (waitall)
+ ret = rxrpc_wait_for_tx_window_nonintr(rx, call);
+ else
+ ret = rxrpc_wait_for_tx_window_intr(rx, call, timeo);
remove_wait_queue(&call->waitq, &myself);
set_current_state(TASK_RUNNING);
@@ -254,7 +308,8 @@ static int rxrpc_send_data(struct rxrpc_sock *rx,
if (msg->msg_flags & MSG_DONTWAIT)
goto maybe_error;
ret = rxrpc_wait_for_tx_window(rx, call,
- &timeo);
+ &timeo,
+ msg->msg_flags & MSG_WAITALL);
if (ret < 0)
goto maybe_error;
}
^ permalink raw reply related
* [PATCH net 0/3] Fix for BPF devmap percpu allocation splat
From: Daniel Borkmann @ 2017-10-17 14:55 UTC (permalink / raw)
To: davem
Cc: tj, ast, john.fastabend, mark.rutland, richard, sp3485, netdev,
linux-kernel, Daniel Borkmann
The set fixes a splat in devmap percpu allocation when we alloc
the flush bitmap. Patch 1 is a prerequisite for the fix in patch 2,
patch 1 is rather small, so if this could be routed via -net, for
example, with Tejun's Ack that would be good. Patch 3 gets rid of
remaining PCPU_MIN_UNIT_SIZE checks, which are percpu allocator
internals and should not be used.
Thanks!
Daniel Borkmann (3):
mm, percpu: add support for __GFP_NOWARN flag
bpf: fix splat for illegal devmap percpu allocation
bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/devmap.c | 5 +++--
kernel/bpf/hashtab.c | 4 ----
mm/percpu.c | 15 ++++++++++-----
4 files changed, 14 insertions(+), 12 deletions(-)
--
1.9.3
^ permalink raw reply
* [PATCH net 1/3] mm, percpu: add support for __GFP_NOWARN flag
From: Daniel Borkmann @ 2017-10-17 14:55 UTC (permalink / raw)
To: davem
Cc: tj, ast, john.fastabend, mark.rutland, richard, sp3485, netdev,
linux-kernel, Daniel Borkmann
In-Reply-To: <cover.1508251210.git.daniel@iogearbox.net>
Add an option for pcpu_alloc() to support __GFP_NOWARN flag.
Currently, we always throw a warning when size or alignment
is unsupported (and also dump stack on failed allocation
requests). The warning itself is harmless since we return
NULL anyway for any failed request, which callers are
required to handle anyway. However, it becomes harmful when
panic_on_warn is set.
The rationale for the WARN() in pcpu_alloc() is that it can
be tracked when larger than supported allocation requests are
made such that allocations limits can be tweaked if warranted.
This makes sense for in-kernel users, however, there are users
of pcpu allocator where allocation size is derived from user
space requests, e.g. when creating BPF maps. In these cases,
the requests should fail gracefully without throwing a splat.
The current work-around was to check allocation size against
the upper limit of PCPU_MIN_UNIT_SIZE from call-sites for
bailing out prior to a call to pcpu_alloc() in order to
avoid throwing the WARN(). This is bad in multiple ways since
PCPU_MIN_UNIT_SIZE is an implementation detail, and having
the checks on call-sites only complicates the code for no
good reason. Thus, lets fix it generically by supporting the
__GFP_NOWARN flag that users can then use with calling the
__alloc_percpu_gfp() helper instead.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: Tejun Heo <tj@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
---
mm/percpu.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/mm/percpu.c b/mm/percpu.c
index aa121ce..a0e0c82 100644
--- a/mm/percpu.c
+++ b/mm/percpu.c
@@ -1329,7 +1329,9 @@ static struct pcpu_chunk *pcpu_chunk_addr_search(void *addr)
* @gfp: allocation flags
*
* Allocate percpu area of @size bytes aligned at @align. If @gfp doesn't
- * contain %GFP_KERNEL, the allocation is atomic.
+ * contain %GFP_KERNEL, the allocation is atomic. If @gfp has __GFP_NOWARN
+ * then no warning will be triggered on invalid or failed allocation
+ * requests.
*
* RETURNS:
* Percpu pointer to the allocated area on success, NULL on failure.
@@ -1337,10 +1339,11 @@ static struct pcpu_chunk *pcpu_chunk_addr_search(void *addr)
static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
gfp_t gfp)
{
+ bool is_atomic = (gfp & GFP_KERNEL) != GFP_KERNEL;
+ bool do_warn = !(gfp & __GFP_NOWARN);
static int warn_limit = 10;
struct pcpu_chunk *chunk;
const char *err;
- bool is_atomic = (gfp & GFP_KERNEL) != GFP_KERNEL;
int slot, off, cpu, ret;
unsigned long flags;
void __percpu *ptr;
@@ -1361,7 +1364,7 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
if (unlikely(!size || size > PCPU_MIN_UNIT_SIZE || align > PAGE_SIZE ||
!is_power_of_2(align))) {
- WARN(true, "illegal size (%zu) or align (%zu) for percpu allocation\n",
+ WARN(do_warn, "illegal size (%zu) or align (%zu) for percpu allocation\n",
size, align);
return NULL;
}
@@ -1482,7 +1485,7 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
fail:
trace_percpu_alloc_percpu_fail(reserved, is_atomic, size, align);
- if (!is_atomic && warn_limit) {
+ if (!is_atomic && do_warn && warn_limit) {
pr_warn("allocation failed, size=%zu align=%zu atomic=%d, %s\n",
size, align, is_atomic, err);
dump_stack();
@@ -1507,7 +1510,9 @@ static void __percpu *pcpu_alloc(size_t size, size_t align, bool reserved,
*
* Allocate zero-filled percpu area of @size bytes aligned at @align. If
* @gfp doesn't contain %GFP_KERNEL, the allocation doesn't block and can
- * be called from any context but is a lot more likely to fail.
+ * be called from any context but is a lot more likely to fail. If @gfp
+ * has __GFP_NOWARN then no warning will be triggered on invalid or failed
+ * allocation requests.
*
* RETURNS:
* Percpu pointer to the allocated area on success, NULL on failure.
--
1.9.3
^ permalink raw reply related
* [PATCH net 2/3] bpf: fix splat for illegal devmap percpu allocation
From: Daniel Borkmann @ 2017-10-17 14:55 UTC (permalink / raw)
To: davem
Cc: tj, ast, john.fastabend, mark.rutland, richard, sp3485, netdev,
linux-kernel, Daniel Borkmann
In-Reply-To: <cover.1508251210.git.daniel@iogearbox.net>
It was reported that syzkaller was able to trigger a splat on
devmap percpu allocation due to illegal/unsupported allocation
request size passed to __alloc_percpu():
[ 70.094249] illegal size (32776) or align (8) for percpu allocation
[ 70.094256] ------------[ cut here ]------------
[ 70.094259] WARNING: CPU: 3 PID: 3451 at mm/percpu.c:1365 pcpu_alloc+0x96/0x630
[...]
[ 70.094325] Call Trace:
[ 70.094328] __alloc_percpu_gfp+0x12/0x20
[ 70.094330] dev_map_alloc+0x134/0x1e0
[ 70.094331] SyS_bpf+0x9bc/0x1610
[ 70.094333] ? selinux_task_setrlimit+0x5a/0x60
[ 70.094334] ? security_task_setrlimit+0x43/0x60
[ 70.094336] entry_SYSCALL_64_fastpath+0x1a/0xa5
This was due to too large max_entries for the map such that we
surpassed the upper limit of PCPU_MIN_UNIT_SIZE. It's fine to
fail naturally here, so switch to __alloc_percpu_gfp() and pass
__GFP_NOWARN instead.
Fixes: 11393cc9b9be ("xdp: Add batching support to redirect map")
Reported-by: Mark Rutland <mark.rutland@arm.com>
Reported-by: Shankara Pailoor <sp3485@columbia.edu>
Reported-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Cc: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/devmap.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index e093d9a..920428d 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -111,8 +111,9 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
err = -ENOMEM;
/* A per cpu bitfield with a bit per possible net device */
- dtab->flush_needed = __alloc_percpu(dev_map_bitmap_size(attr),
- __alignof__(unsigned long));
+ dtab->flush_needed = __alloc_percpu_gfp(dev_map_bitmap_size(attr),
+ __alignof__(unsigned long),
+ GFP_KERNEL | __GFP_NOWARN);
if (!dtab->flush_needed)
goto free_dtab;
--
1.9.3
^ permalink raw reply related
* [PATCH net 3/3] bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
From: Daniel Borkmann @ 2017-10-17 14:55 UTC (permalink / raw)
To: davem
Cc: tj, ast, john.fastabend, mark.rutland, richard, sp3485, netdev,
linux-kernel, Daniel Borkmann
In-Reply-To: <cover.1508251210.git.daniel@iogearbox.net>
PCPU_MIN_UNIT_SIZE is an implementation detail of the percpu
allocator. Given we support __GFP_NOWARN now, lets just let
the allocation request fail naturally instead. The two call
sites from BPF mistakenly assumed __GFP_NOWARN would work, so
no changes needed to their actual __alloc_percpu_gfp() calls
which use the flag already.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/hashtab.c | 4 ----
2 files changed, 1 insertion(+), 5 deletions(-)
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 98c0f00..e263673 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -98,7 +98,7 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
array_size += (u64) attr->max_entries * elem_size * num_possible_cpus();
if (array_size >= U32_MAX - PAGE_SIZE ||
- elem_size > PCPU_MIN_UNIT_SIZE || bpf_array_alloc_percpu(array)) {
+ bpf_array_alloc_percpu(array)) {
bpf_map_area_free(array);
return ERR_PTR(-ENOMEM);
}
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 431126f..6533f08 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -317,10 +317,6 @@ static struct bpf_map *htab_map_alloc(union bpf_attr *attr)
*/
goto free_htab;
- if (percpu && round_up(htab->map.value_size, 8) > PCPU_MIN_UNIT_SIZE)
- /* make sure the size for pcpu_alloc() is reasonable */
- goto free_htab;
-
htab->elem_size = sizeof(struct htab_elem) +
round_up(htab->map.key_size, 8);
if (percpu)
--
1.9.3
^ permalink raw reply related
* Re: RFC(v2): Audit Kernel Container IDs
From: Casey Schaufler @ 2017-10-17 14:59 UTC (permalink / raw)
To: Simo Sorce, Steve Grubb, linux-audit
Cc: Richard Guy Briggs, mszeredi, Eric W. Biederman, jlayton,
Carlos O'Donell, Linux API, Linux Containers, Linux Kernel,
Eric Paris, David Howells, Al Viro, Andy Lutomirski,
Linux Network Development, Linux FS Devel, cgroups,
Serge E. Hallyn, trondmy
In-Reply-To: <1508243469.6230.24.camel@redhat.com>
On 10/17/2017 5:31 AM, Simo Sorce wrote:
> On Mon, 2017-10-16 at 21:42 -0400, Steve Grubb wrote:
>> On Monday, October 16, 2017 8:33:40 PM EDT Richard Guy Briggs wrote:
>>> There is such a thing, but the kernel doesn't know about it
>>> yet. This same situation exists for loginuid and sessionid which
>>> are userspace concepts that the kernel tracks for the convenience
>>> of userspace. As for its name, I'm not particularly picky, so if
>>> you don't like CAP_CONTAINER_* then I'm fine with
>>> CAP_AUDIT_CONTAINERID. It really needs to be distinct from
>>> CAP_AUDIT_WRITE and CAP_AUDIT_CONTROL since we don't want to give
>>> the ability to set a containerID to any process that is able to do
>>> audit logging (such as vsftpd) and similarly we don't want to give
>>> the orchestrator the ability to control the setup of the audit
>>> daemon.
>> A long time ago, we were debating what should guard against rouge
>> processes from setting the loginuid. Casey argued that the ability to
>> set the loginuid means they have the ability to control the audit
>> trail. That means that it should be guarded by CAP_AUDIT_CONTROL. I
>> think the same logic applies today.
> The difference is that with loginuid you needed to give processes able
> to audit also the ability to change it. You do not want to tie the
> ability to change container ids to the ability to audit. You want to be
> able to do audit stuff (within the container) without allowing it to
> change the container id.
Without a *kernel* policy on containerIDs you can't say what
security policy is being exempted. Without that you can't say what
capability is (or isn't) appropriate. You need a reason to have
a capability check that makes sense in the context of the kernel
security policy. Since we don't know what a container is in the
kernel, that's pretty hard. We don't create "fuzzy" capabilities
based on the trendy application behavior of the moment. If the
behavior is not related it audit, there's no reason for it, and
if it is, CAP_AUDIT_CONTROL works just fine. If this doesn't work
in your application security model I suggest that is where you
need to make changes.
> Of course if we made container id a write-once property maybe there is
> no need for controls at all, but I'm pretty sure there will be
> situations where write-once may not be usable in practice.
>
>> The ability to arbitrarily set a container ID means the process has
>> the ability to indirectly control the audit trail.
> The container Id can be used also for authorization purposes (by other
> processes on the host), not just audit, I think this is why a separate
> control has been proposed.
>
> Simo.
>
^ permalink raw reply
* [PATCH][next] mqprio: fix potential null pointer dereference on opt
From: Colin King @ 2017-10-17 15:01 UTC (permalink / raw)
To: Jamal Hadi Salim, Cong Wang, Jiri Pirko, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The pointer opt has a null check however before for this check opt is
dereferenced when len is initialized, hence we potentially have a null
pointer deference on opt. Avoid this by checking for a null opt before
dereferencing it.
Detected by CoverityScan, CID#1458234 ("Dereference before null check")
Fixes: 4e8b86c06269 ("mqprio: Introduce new hardware offload mode and shaper in mqprio")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
net/sched/sch_mqprio.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
index f1ae9be83934..37d5fd9b617c 100644
--- a/net/sched/sch_mqprio.c
+++ b/net/sched/sch_mqprio.c
@@ -142,7 +142,7 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
struct nlattr *tb[TCA_MQPRIO_MAX + 1];
struct nlattr *attr;
int rem;
- int len = nla_len(opt) - NLA_ALIGN(sizeof(*qopt));
+ int len;
BUILD_BUG_ON(TC_MAX_QUEUE != TC_QOPT_MAX_QUEUE);
BUILD_BUG_ON(TC_BITMASK != TC_QOPT_BITMASK);
@@ -160,6 +160,7 @@ static int mqprio_init(struct Qdisc *sch, struct nlattr *opt)
if (mqprio_parse_opt(dev, qopt))
return -EINVAL;
+ len = nla_len(opt) - NLA_ALIGN(sizeof(*qopt));
if (len > 0) {
err = parse_attr(tb, TCA_MQPRIO_MAX, opt, mqprio_policy,
sizeof(*qopt));
--
2.14.1
^ permalink raw reply related
* RE: [PATCH net 0/3] Fix for BPF devmap percpu allocation splat
From: David Laight @ 2017-10-17 15:03 UTC (permalink / raw)
To: 'Daniel Borkmann', davem@davemloft.net
Cc: tj@kernel.org, ast@kernel.org, john.fastabend@gmail.com,
mark.rutland@arm.com, richard@nod.at, sp3485@columbia.edu,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <cover.1508251210.git.daniel@iogearbox.net>
From: Daniel Borkmann
> Sent: 17 October 2017 15:56
>
> The set fixes a splat in devmap percpu allocation when we alloc
> the flush bitmap. Patch 1 is a prerequisite for the fix in patch 2,
> patch 1 is rather small, so if this could be routed via -net, for
> example, with Tejun's Ack that would be good. Patch 3 gets rid of
> remaining PCPU_MIN_UNIT_SIZE checks, which are percpu allocator
> internals and should not be used.
Does it make sense to allow the user program to try to allocate ever
smaller very large maps until it finds one that succeeds - thus
using up all the percpu space?
Or is this a 'root only' 'shoot self in foot' job?
David
^ permalink raw reply
* Re: [PATCH net 0/3] Fix for BPF devmap percpu allocation splat
From: Daniel Borkmann @ 2017-10-17 15:11 UTC (permalink / raw)
To: David Laight, davem@davemloft.net
Cc: tj@kernel.org, ast@kernel.org, john.fastabend@gmail.com,
mark.rutland@arm.com, richard@nod.at, sp3485@columbia.edu,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6DD009980D@AcuExch.aculab.com>
On 10/17/2017 05:03 PM, David Laight wrote:
> From: Daniel Borkmann
>> Sent: 17 October 2017 15:56
>>
>> The set fixes a splat in devmap percpu allocation when we alloc
>> the flush bitmap. Patch 1 is a prerequisite for the fix in patch 2,
>> patch 1 is rather small, so if this could be routed via -net, for
>> example, with Tejun's Ack that would be good. Patch 3 gets rid of
>> remaining PCPU_MIN_UNIT_SIZE checks, which are percpu allocator
>> internals and should not be used.
>
> Does it make sense to allow the user program to try to allocate ever
> smaller very large maps until it finds one that succeeds - thus
> using up all the percpu space?
>
> Or is this a 'root only' 'shoot self in foot' job?
It's root only although John still has a pending fix to be flushed
out for -net first in the next days to actually enforce that cap
(devmap is not in an official kernel yet at this point, so all good),
but apart from this, all map allocs in general are accounted for
as well.
Thanks,
Daniel
^ permalink raw reply
* Re: [PATCH, net-next] i40e: avoid 64-bit division where possible
From: Jeff Kirsher @ 2017-10-17 15:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jacob Keller, Mitch Williams, Alexander Duyck, Amritha Nambiar,
Filip Sadowski, David S. Miller, Björn Töpel,
intel-wired-lan, netdev, linux-kernel
In-Reply-To: <20171017102351.492492-1-arnd@arndb.de>
[-- Attachment #1: Type: text/plain, Size: 1208 bytes --]
On Tue, 2017-10-17 at 12:23 +0200, Arnd Bergmann wrote:
> The new bandwidth calculation causes a link error on 32-bit
> architectures, like
>
> ERROR: "__aeabi_uldivmod" [drivers/net/ethernet/intel/i40e/i40e.ko]
> undefined!
>
> The problem is the max_tx_rate calculation that uses 64-bit integers.
> This is not really necessary since the numbers are in MBit/s so
> they won't be higher than 40000 for the highest support rate, and
> are guaranteed to not exceed 2^32 in future generations either.
>
> This changes the representation to 'u32' when dealing with MBit/s
> and uses div_u64() to convert from u64 numbers in byte/s.
>
> Fixes: 2027d4deacb1 ("i40e: Add support setting TC max bandwidth
> rates")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/net/ethernet/intel/i40e/i40e.h | 4 ++--
> drivers/net/ethernet/intel/i40e/i40e_main.c | 27 ++++++++++++++-----
> --------
> 2 files changed, 16 insertions(+), 15 deletions(-)
Unfortunately your patch does not apply cleanly to my tree. Arnd,
could you please rebase your patch based my next-queue tree (dev-queue
branch)? I already have several i40e patches queued up and applied to
that branch.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH net] sctp: do not peel off an assoc from one netns to another one
From: Xin Long @ 2017-10-17 15:26 UTC (permalink / raw)
To: network dev, linux-sctp
Cc: davem, Marcelo Ricardo Leitner, Neil Horman, chunwang, syzkaller
Now when peeling off an association to the sock in another netns, all
transports in this assoc are not to be rehashed and keep use the old
key in hashtable.
As a transport uses sk->net as the hash key to insert into hashtable,
it would miss removing these transports from hashtable due to the new
netns when closing the sock and all transports are being freeed, then
later an use-after-free issue could be caused when looking up an asoc
and dereferencing those transports.
This is a very old issue since very beginning, ChunYu found it with
syzkaller fuzz testing with this series:
socket$inet6_sctp()
bind$inet6()
sendto$inet6()
unshare(0x40000000)
getsockopt$inet_sctp6_SCTP_GET_ASSOC_ID_LIST()
getsockopt$inet_sctp6_SCTP_SOCKOPT_PEELOFF()
This patch is to block this call when peeling one assoc off from one
netns to another one, so that the netns of all transport would not
go out-sync with the key in hashtable.
Note that this patch didn't fix it by rehashing transports, as it's
difficult to handle the situation when the tuple is already in use
in the new netns. Besides, no one would like to peel off one assoc
to another netns, considering ipaddrs, ifaces, etc. are usually
different.
Reported-by: ChunYu Wang <chunwang@redhat.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
net/sctp/socket.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index d4730ad..17841ab 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4906,6 +4906,10 @@ int sctp_do_peeloff(struct sock *sk, sctp_assoc_t id, struct socket **sockp)
struct socket *sock;
int err = 0;
+ /* Do not peel off from one netns to another one. */
+ if (!net_eq(current->nsproxy->net_ns, sock_net(sk)))
+ return -EINVAL;
+
if (!asoc)
return -EINVAL;
--
2.1.0
^ permalink raw reply related
* Re: RFC(v2): Audit Kernel Container IDs
From: Simo Sorce @ 2017-10-17 15:28 UTC (permalink / raw)
To: Casey Schaufler, Steve Grubb, linux-audit-H+wXaHxf7aLQT0dZR+AlfA
Cc: mszeredi-H+wXaHxf7aLQT0dZR+AlfA, trondmy-7I+n7zu2hftEKMMhf/gKZA,
jlayton-H+wXaHxf7aLQT0dZR+AlfA, Linux API, Linux Containers,
Linux Kernel, David Howells, Carlos O'Donell,
cgroups-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
Andy Lutomirski, Linux Network Development, Linux FS Devel,
Eric Paris, Al Viro
In-Reply-To: <a07968f6-fef1-f49d-01f1-6c660c0ada20-iSGtlc1asvQWG2LlvL+J4A@public.gmane.org>
On Tue, 2017-10-17 at 07:59 -0700, Casey Schaufler wrote:
> On 10/17/2017 5:31 AM, Simo Sorce wrote:
> > On Mon, 2017-10-16 at 21:42 -0400, Steve Grubb wrote:
> > > On Monday, October 16, 2017 8:33:40 PM EDT Richard Guy Briggs
> > > wrote:
> > > > There is such a thing, but the kernel doesn't know about it
> > > > yet. This same situation exists for loginuid and sessionid
> > > > which
> > > > are userspace concepts that the kernel tracks for the
> > > > convenience
> > > > of userspace. As for its name, I'm not particularly picky, so
> > > > if
> > > > you don't like CAP_CONTAINER_* then I'm fine with
> > > > CAP_AUDIT_CONTAINERID. It really needs to be distinct from
> > > > CAP_AUDIT_WRITE and CAP_AUDIT_CONTROL since we don't want to
> > > > give
> > > > the ability to set a containerID to any process that is able to
> > > > do
> > > > audit logging (such as vsftpd) and similarly we don't want to
> > > > give
> > > > the orchestrator the ability to control the setup of the audit
> > > > daemon.
> > >
> > > A long time ago, we were debating what should guard against rouge
> > > processes from setting the loginuid. Casey argued that the
> > > ability to
> > > set the loginuid means they have the ability to control the audit
> > > trail. That means that it should be guarded by CAP_AUDIT_CONTROL.
> > > I
> > > think the same logic applies today.
> >
> > The difference is that with loginuid you needed to give processes
> > able
> > to audit also the ability to change it. You do not want to tie the
> > ability to change container ids to the ability to audit. You want
> > to be
> > able to do audit stuff (within the container) without allowing it
> > to
> > change the container id.
>
> Without a *kernel* policy on containerIDs you can't say what
> security policy is being exempted.
The policy has been basically stated earlier.
A way to track a set of processes from a specific point in time
forward. The name used is "container id", but it could be anything.
This marker is mostly used by user space to track process hierarchies
without races, these processes can be very privileged, and must not be
allowed to change the marker themselves when granted the current common
capabilities.
Is this a good enough description ? If not can you clarify your
expectations ?
> Without that you can't say what capability is (or isn't)
> appropriate.
See if the above is sufficient please.
> You need a reason to have a capability check that makes sense in the
> context of the kernel security policy.
I think the proposal had a reason, we may debate on whether that reason
is good enough.
> Since we don't know what a container is in the kernel,
Please do not fixate on the word container.
> that's pretty hard. We don't create "fuzzy" capabilities
> based on the trendy application behavior of the moment. If the
> behavior is not related it audit, there's no reason for it, and
> if it is, CAP_AUDIT_CONTROL works just fine. If this doesn't work
> in your application security model I suggest that is where you
> need to make changes.
The authors of the proposal came to the conclusion that kernel
assistance is needed. It would be nice to discuss the merits of it.
If you do not understand why the request has been made it would be more
useful to ask specific questions to understand what and why is the ask.
Pushing back is fine, if you have understood the problem and have valid
arguments against a kernel level solution (and possibly suggestions for
a working user space solution), otherwise you are not adding value to
the discussion.
Simo.
--
Simo Sorce
Sr. Principal Software Engineer
Red Hat, Inc
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
^ permalink raw reply
* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Xin Long @ 2017-10-17 15:31 UTC (permalink / raw)
To: Eric Dumazet
Cc: Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, netdev,
Wei Wang, Eric Dumazet, linux-sctp
In-Reply-To: <1508247956.31614.103.camel@edumazet-glaptop3.roam.corp.google.com>
On Tue, Oct 17, 2017 at 9:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> SCTP experts.
>
> syszkaller reported a few crashes in sctp_packet_config() with invalid
> access to a deleted dst.
>
> The rcu_read_lock() in sctp_packet_config() is suspect.
>
> It does not protect anything at the moment.
>
> If we expect tp->dst to be manipulated/changed by another cpu/thread,
> then we need proper rcu protection.
>
> Following patch to show what would be a minimal change (but obviously
> bigger changes are needed, like sctp_transport_pmtu_check() and
> sctp_transport_dst_check(), and proper sparse annotations)
will check all places accessing tp->dst in sctp.
>
>
> BTW, sparse throws a lot of errors, any volunteer to clean this mess ?
will do it.
Thanks for reporting this.
>
> make C=2 M=net/sctp
>
> Thanks.
>
> diff --git a/net/sctp/output.c b/net/sctp/output.c
> index 4a865cd06d76cd5b2aa417de618da3203f7b53e4..d7f320f5acc271189ec9474795b6ececed7ad2b9 100644
> --- a/net/sctp/output.c
> +++ b/net/sctp/output.c
> @@ -86,6 +86,7 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
> {
> struct sctp_transport *tp = packet->transport;
> struct sctp_association *asoc = tp->asoc;
> + struct dst_entry *dst;
> struct sock *sk;
>
> pr_debug("%s: packet:%p vtag:0x%x\n", __func__, packet, vtag);
> @@ -121,17 +122,15 @@ void sctp_packet_config(struct sctp_packet *packet, __u32 vtag,
> sctp_packet_append_chunk(packet, chunk);
> }
>
> - if (!tp->dst)
> - return;
> -
> /* set packet max_size with gso_max_size if gso is enabled*/
> rcu_read_lock();
> - if (__sk_dst_get(sk) != tp->dst) {
> - dst_hold(tp->dst);
> - sk_setup_caps(sk, tp->dst);
> + dst = rcu_dereference(tp->dst);
> + if (dst) {
> + if (__sk_dst_get(sk) != dst && dst_hold_safe(dst))
> + sk_setup_caps(sk, dst);
> + packet->max_size = sk_can_gso(sk) ? dst->dev->gso_max_size
> + : asoc->pathmtu;
> }
> - packet->max_size = sk_can_gso(sk) ? tp->dst->dev->gso_max_size
> - : asoc->pathmtu;
> rcu_read_unlock();
> }
>
>
>
^ permalink raw reply
* Re: [PATCH net-next] tcp: Check daddr_cache before use in tracepoint
From: David Ahern @ 2017-10-17 15:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev, xiyou.wangcong
In-Reply-To: <1508211908.31614.90.camel@edumazet-glaptop3.roam.corp.google.com>
On 10/16/17 9:45 PM, Eric Dumazet wrote:
> IPV6 TCP uses sk->sk_v6_daddr and sk->->sk_v6_rcv_saddr
I moved v2 to those.
> So I would rather remove the need to fetch np = inet6_sk(sk) in the
> first place, and look at sk->sk_family instead.
I'll spin a v3 with that change.
^ permalink raw reply
* Re: RFC(v2): Audit Kernel Container IDs
From: James Bottomley @ 2017-10-17 15:44 UTC (permalink / raw)
To: Simo Sorce, Casey Schaufler, Steve Grubb,
linux-audit-H+wXaHxf7aLQT0dZR+AlfA
Cc: mszeredi-H+wXaHxf7aLQT0dZR+AlfA, trondmy-7I+n7zu2hftEKMMhf/gKZA,
jlayton-H+wXaHxf7aLQT0dZR+AlfA, Linux API, Linux Containers,
Linux Kernel, David Howells, Carlos O'Donell,
cgroups-u79uwXL29TY76Z2rM5mHXA, Eric W. Biederman,
Andy Lutomirski, Linux Network Development, Linux FS Devel,
Eric Paris, Al Viro
In-Reply-To: <1508254120.6230.34.camel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Tue, 2017-10-17 at 11:28 -0400, Simo Sorce wrote:
> > Without a *kernel* policy on containerIDs you can't say what
> > security policy is being exempted.
>
> The policy has been basically stated earlier.
>
> A way to track a set of processes from a specific point in time
> forward. The name used is "container id", but it could be anything.
> This marker is mostly used by user space to track process hierarchies
> without races, these processes can be very privileged, and must not
> be allowed to change the marker themselves when granted the current
> common capabilities.
>
> Is this a good enough description ? If not can you clarify your
> expectations ?
I think you mean you want to be able to apply a label to a process
which is inherited across forks. The label should only be susceptible
to modification by something possessing a capability (which one TBD).
The idea is that processes spawned into a container would be labelled
by the container orchestration system. It's unclear what should happen
to processes using nsenter after the fact, but policy for that should
be up to the orchestration system.
The label will be used as a tag for audit information.
I think you were missing label inheritance above.
The security implications are that anything that can change the label
could also hide itself and its doings from the audit system and thus
would be used as a means to evade detection. I actually think this
means the label should be write once (once you've set it, you can't
change it) and orchestration systems should begin as unlabelled
processes allowing them to do arbitrary forks.
For nested containers, I actually think the label should be
hierarchical, so you can add a label for the new nested container but
it still also contains its parents label as well.
James
^ 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