Netdev List
 help / color / mirror / Atom feed
* 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

* Re: [PATCH net-next 1/4] bpf: split verifier and program ops
From: Alexei Starovoitov @ 2017-10-17 15:44 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
In-Reply-To: <20171016234056.1964-2-jakub.kicinski@netronome.com>

On Mon, Oct 16, 2017 at 04:40:53PM -0700, Jakub Kicinski wrote:
> struct bpf_verifier_ops contains both verifier ops and operations
> used later during program's lifetime (test_run).  Split the runtime
> ops into a different structure.
> 
> BPF_PROG_TYPE() will now append ## _prog_ops or ## _verifier_ops
> to the names.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH net-next 2/4] bpf: remove the verifier ops from program structure
From: Alexei Starovoitov @ 2017-10-17 15:45 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
In-Reply-To: <20171016234056.1964-3-jakub.kicinski@netronome.com>

On Mon, Oct 16, 2017 at 04:40:54PM -0700, Jakub Kicinski wrote:
> Since the verifier ops don't have to be associated with
> the program for its entire lifetime we can move it to
> verifier's struct bpf_verifier_env.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH net-next 3/4] bpf: move knowledge about post-translation offsets out of verifier
From: Alexei Starovoitov @ 2017-10-17 15:45 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
In-Reply-To: <20171016234056.1964-4-jakub.kicinski@netronome.com>

On Mon, Oct 16, 2017 at 04:40:55PM -0700, Jakub Kicinski wrote:
> Use the fact that verifier ops are now separate from program
> ops to define a separate set of callbacks for verification of
> already translated programs.
> 
> Since we expect the analyzer ops to be defined only for
> a small subset of all program types initialize their array
> by hand (don't use linux/bpf_types.h).
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH net-next 4/4] bpf: allow access to skb->len from offloads
From: Alexei Starovoitov @ 2017-10-17 15:46 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: netdev, oss-drivers, daniel
In-Reply-To: <20171016234056.1964-5-jakub.kicinski@netronome.com>

On Mon, Oct 16, 2017 at 04:40:56PM -0700, Jakub Kicinski wrote:
> Since we are now doing strict checking of what offloads
> may access, make sure skb->len is on that list.
> 
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH, net-next] i40e: avoid 64-bit division where possible
From: Arnd Bergmann @ 2017-10-17 15:46 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: Jacob Keller, Mitch Williams, Alexander Duyck, Amritha Nambiar,
	Filip Sadowski, David S. Miller, Björn Töpel,
	intel-wired-lan, Networking, Linux Kernel Mailing List
In-Reply-To: <1508253294.2928.2.camel@intel.com>

On Tue, Oct 17, 2017 at 5:14 PM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> 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!

> 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.

I see you already applied a fix from Alan Brady on that branch. I think
his version is sufficient to avoid the build problems, but mine is better.
I've rebased my patch now to revert parts of his fix. Please decide for
yourself whether you want to apply it on top, or are happy enough with
the existing version.

       Arnd

^ permalink raw reply

* [PATCH] [v2] i40e: avoid 64-bit division where possible
From: Arnd Bergmann @ 2017-10-17 15:49 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: Arnd Bergmann, Alan Brady, Jacob Keller, Amritha Nambiar,
	Mitch Williams, Alexander Duyck, Filip Sadowski, David S. Miller,
	Björn Töpel, intel-wired-lan, netdev, linux-kernel

The new bandwidth calculation caused 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.

Another patch from Alan Brady fixed the link error by adding
many calls to do_div(), which makes the code less efficent and
less readable than necessary.

This changes the representation to 'u32' when dealing with MBit/s
and uses div_u64() to convert from u64 numbers in byte/s, reverting
parts of Alan's earlier fix that have become obsolete now.

Fixes: 2027d4deacb1 ("i40e: Add support setting TC max bandwidth rates")
Fixes: 73983b5ae011 ("i40e: fix u64 division usage")
Cc: Alan Brady <alan.brady@intel.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/intel/i40e/i40e.h      |  4 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c | 70 +++++++++++------------------
 2 files changed, 27 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e.h b/drivers/net/ethernet/intel/i40e/i40e.h
index c3f13120f3ce..c7aa0c982273 100644
--- a/drivers/net/ethernet/intel/i40e/i40e.h
+++ b/drivers/net/ethernet/intel/i40e/i40e.h
@@ -407,7 +407,7 @@ struct i40e_channel {
 	u8 enabled_tc;
 	struct i40e_aqc_vsi_properties_data info;
 
-	u64 max_tx_rate;
+	u32 max_tx_rate; /* in Mbits/s */
 
 	/* track this channel belongs to which VSI */
 	struct i40e_vsi *parent_vsi;
@@ -1101,5 +1101,5 @@ static inline bool i40e_enabled_xdp_vsi(struct i40e_vsi *vsi)
 }
 
 int i40e_create_queue_channel(struct i40e_vsi *vsi, struct i40e_channel *ch);
-int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate);
+int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate);
 #endif /* _I40E_H_ */
diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 3ceda140170d..57682cc78508 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -5448,17 +5448,16 @@ int i40e_get_link_speed(struct i40e_vsi *vsi)
  *
  * Helper function to set BW limit for a given VSI
  **/
-int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
+int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u32 max_tx_rate)
 {
 	struct i40e_pf *pf = vsi->back;
-	u64 credits = 0;
 	int speed = 0;
 	int ret = 0;
 
 	speed = i40e_get_link_speed(vsi);
 	if (max_tx_rate > speed) {
 		dev_err(&pf->pdev->dev,
-			"Invalid max tx rate %llu specified for VSI seid %d.",
+			"Invalid max tx rate %u specified for VSI seid %d.",
 			max_tx_rate, seid);
 		return -EINVAL;
 	}
@@ -5469,13 +5468,12 @@ int i40e_set_bw_limit(struct i40e_vsi *vsi, u16 seid, u64 max_tx_rate)
 	}
 
 	/* Tx rate credits are in values of 50Mbps, 0 is disabled */
-	credits = max_tx_rate;
-	do_div(credits, I40E_BW_CREDIT_DIVISOR);
-	ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid, credits,
+	ret = i40e_aq_config_vsi_bw_limit(&pf->hw, seid,
+					  max_tx_rate / I40E_BW_CREDIT_DIVISOR,
 					  I40E_MAX_BW_INACTIVE_ACCUM, NULL);
 	if (ret)
 		dev_err(&pf->pdev->dev,
-			"Failed set tx rate (%llu Mbps) for vsi->seid %u, err %s aq_err %s\n",
+			"Failed set tx rate (%u Mbps) for vsi->seid %u, err %s aq_err %s\n",
 			max_tx_rate, seid, i40e_stat_str(&pf->hw, ret),
 			i40e_aq_str(&pf->hw, pf->hw.aq.asq_last_status));
 	return ret;
@@ -6158,17 +6156,13 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
 
 	/* configure VSI for BW limit */
 	if (ch->max_tx_rate) {
-		u64 credits = ch->max_tx_rate;
-
 		if (i40e_set_bw_limit(vsi, ch->seid, ch->max_tx_rate))
 			return -EINVAL;
 
-		do_div(credits, I40E_BW_CREDIT_DIVISOR);
 		dev_dbg(&pf->pdev->dev,
-			"Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
+			"Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
 			ch->max_tx_rate,
-			credits,
-			ch->seid);
+			ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR, ch->seid);
 	}
 
 	/* in case of VF, this will be main SRIOV VSI */
@@ -6189,7 +6183,6 @@ int i40e_create_queue_channel(struct i40e_vsi *vsi,
 static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
 {
 	struct i40e_channel *ch;
-	u64 max_rate = 0;
 	int ret = 0, i;
 
 	/* Create app vsi with the TCs. Main VSI with TC0 is already set up */
@@ -6211,9 +6204,8 @@ static int i40e_configure_queue_channels(struct i40e_vsi *vsi)
 			/* Bandwidth limit through tc interface is in bytes/s,
 			 * change to Mbit/s
 			 */
-			max_rate = vsi->mqprio_qopt.max_rate[i];
-			do_div(max_rate, I40E_BW_MBPS_DIVISOR);
-			ch->max_tx_rate = max_rate;
+			ch->max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[i],
+						  I40E_BW_CREDIT_DIVISOR);
 
 			list_add_tail(&ch->list, &vsi->ch_list);
 
@@ -6643,7 +6635,6 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
 				     struct tc_mqprio_qopt_offload *mqprio_qopt)
 {
 	u64 sum_max_rate = 0;
-	u64 max_rate = 0;
 	int i;
 
 	if (mqprio_qopt->qopt.offset[0] != 0 ||
@@ -6658,9 +6649,8 @@ static int i40e_validate_mqprio_qopt(struct i40e_vsi *vsi,
 				"Invalid min tx rate (greater than 0) specified\n");
 			return -EINVAL;
 		}
-		max_rate = mqprio_qopt->max_rate[i];
-		do_div(max_rate, I40E_BW_MBPS_DIVISOR);
-		sum_max_rate += max_rate;
+		sum_max_rate += div_u64(mqprio_qopt->max_rate[i],
+					I40E_BW_CREDIT_DIVISOR);
 
 		if (i >= mqprio_qopt->qopt.num_tc - 1)
 			break;
@@ -6804,18 +6794,14 @@ static int i40e_setup_tc(struct net_device *netdev, void *type_data)
 
 	if (pf->flags & I40E_FLAG_TC_MQPRIO) {
 		if (vsi->mqprio_qopt.max_rate[0]) {
-			u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
-
-			do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
+			u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
+						  I40E_BW_CREDIT_DIVISOR);
 			ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
 			if (!ret) {
-				u64 credits = max_tx_rate;
-
-				do_div(credits, I40E_BW_CREDIT_DIVISOR);
 				dev_dbg(&vsi->back->pdev->dev,
-					"Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
+					"Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
 					max_tx_rate,
-					credits,
+					max_tx_rate / I40E_BW_CREDIT_DIVISOR,
 					vsi->seid);
 			} else {
 				need_reset = true;
@@ -9024,15 +9010,12 @@ static int i40e_rebuild_channels(struct i40e_vsi *vsi)
 			return ret;
 		}
 		if (ch->max_tx_rate) {
-			u64 credits = ch->max_tx_rate;
-
 			if (i40e_set_bw_limit(vsi, ch->seid,
 					      ch->max_tx_rate))
 				return -EINVAL;
 
-			do_div(credits, I40E_BW_CREDIT_DIVISOR);
 			dev_dbg(&vsi->back->pdev->dev,
-				"Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
+				"Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
 				ch->max_tx_rate,
 				ch->max_tx_rate / I40E_BW_CREDIT_DIVISOR,
 				ch->seid);
@@ -9314,21 +9297,18 @@ static void i40e_rebuild(struct i40e_pf *pf, bool reinit, bool lock_acquired)
 	}
 
 	if (vsi->mqprio_qopt.max_rate[0]) {
-		u64 max_tx_rate = vsi->mqprio_qopt.max_rate[0];
-		u64 credits = 0;
+		u32 max_tx_rate = div_u64(vsi->mqprio_qopt.max_rate[0],
+					  I40E_BW_CREDIT_DIVISOR);
 
-		do_div(max_tx_rate, I40E_BW_MBPS_DIVISOR);
 		ret = i40e_set_bw_limit(vsi, vsi->seid, max_tx_rate);
-		if (ret)
+		if (!ret)
+			dev_dbg(&vsi->back->pdev->dev,
+				"Set tx rate of %u Mbps (count of 50Mbps %u) for vsi->seid %u\n",
+				max_tx_rate,
+				max_tx_rate / I40E_BW_CREDIT_DIVISOR,
+				vsi->seid);
+		else
 			goto end_unlock;
-
-		credits = max_tx_rate;
-		do_div(credits, I40E_BW_CREDIT_DIVISOR);
-		dev_dbg(&vsi->back->pdev->dev,
-			"Set tx rate of %llu Mbps (count of 50Mbps %llu) for vsi->seid %u\n",
-			max_tx_rate,
-			credits,
-			vsi->seid);
 	}
 
 	ret = i40e_rebuild_cloud_filters(vsi, vsi->seid);
-- 
2.9.0

^ permalink raw reply related

* Re: [patch net v2 1/4] net/sched: Change tc_action refcnt and bindcnt to atomic
From: Cong Wang @ 2017-10-17 15:52 UTC (permalink / raw)
  To: Chris Mi
  Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Lucas Bates,
	Jiri Pirko, David Miller
In-Reply-To: <VI1PR0501MB21439CB952C540F903969648AB4C0@VI1PR0501MB2143.eurprd05.prod.outlook.com>

On Mon, Oct 16, 2017 at 6:14 PM, Chris Mi <chrism@mellanox.com> wrote:
> I don't think this bug were introduced by above two commits only.
> Actually, this bug were introduced by several commits, at least the following:
> 1. refcnt and bindcnt are not atomic

Nope, it is perfectly okay with non-atomic as long as no parallel,
and without RCU callback they are perfectly serialized by RTNL.


> 2. passing actions using list instead of arrays (I think initially we are using arrays)

We are discussing patch 1/4, this is patch 2/4, so irrelevant.


> 3. using RCU callbacks

This introduces problem 1.


> So instead of blaming the latest commit, it is better to say it is a pre-git error.

You are wrong.

^ permalink raw reply

* Re: [PATCH net 1/3] mm, percpu: add support for __GFP_NOWARN flag
From: Alexei Starovoitov @ 2017-10-17 15:53 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, tj, ast, john.fastabend, mark.rutland, richard, sp3485,
	netdev, linux-kernel
In-Reply-To: <beeeab23cda20c0b0ee3e4ccb30b91518de6c075.1508251210.git.daniel@iogearbox.net>

On Tue, Oct 17, 2017 at 04:55:52PM +0200, Daniel Borkmann wrote:
> 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>

The approach looks great to me. We've been doing this dance around
allocator warning for long time. It's really not a job of bpf code
to guess into valid parameters of pcpu alloc.
Adding support for __GFP_NOWARN and using it in bpf is much cleaner
fix that avoids layering violations.

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH net 2/3] bpf: fix splat for illegal devmap percpu allocation
From: Alexei Starovoitov @ 2017-10-17 15:54 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, tj, ast, john.fastabend, mark.rutland, richard, sp3485,
	netdev, linux-kernel
In-Reply-To: <dd091b1fec763d74185a156ecd2f972810bdd1ce.1508251210.git.daniel@iogearbox.net>

On Tue, Oct 17, 2017 at 04:55:53PM +0200, Daniel Borkmann wrote:
> 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>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH net 3/3] bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
From: Alexei Starovoitov @ 2017-10-17 15:55 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: davem, tj, ast, john.fastabend, mark.rutland, richard, sp3485,
	netdev, linux-kernel
In-Reply-To: <a11e92b8ba4f5270437caa629307c300bca37976.1508251210.git.daniel@iogearbox.net>

On Tue, Oct 17, 2017 at 04:55:54PM +0200, Daniel Borkmann wrote:
> 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>

Acked-by: Alexei Starovoitov <ast@kernel.org>

^ permalink raw reply

* Re: [PATCH] net: export netdev_txq_to_tc to allow sch_mqprio to compile as module
From: David Miller @ 2017-10-17 16:00 UTC (permalink / raw)
  To: henrik
  Cc: netdev, edumazet, daniel, dsahern, alexander.h.duyck, willemb,
	john.fastabend, me, linux-kernel, haustad, jesus.sanchez-palencia
In-Reply-To: <1508235010-4056-1-git-send-email-haustad@cisco.com>

From: Henrik Austad <henrik@austad.us>
Date: Tue, 17 Oct 2017 12:10:10 +0200

> 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>

Applied, thanks.

^ permalink raw reply

* Re: RFC(v2): Audit Kernel Container IDs
From: Casey Schaufler @ 2017-10-17 16:10 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: <1508254120.6230.34.camel@redhat.com>

On 10/17/2017 8:28 AM, Simo Sorce wrote:
> 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.

No. The expected user space behavior has been stated.

> 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.

Then you want Jose Bollo's PTAGS. It's insane to add yet another
arbitrary ID to the task for a special purpose. Add a general tagging
mechanism instead. We could add a gazillion new id's, each with it's
own capability if we head down this road.

> 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.

Let's be clear. What happens in user space stays in user space.
The kernel does not give a fig about user space policy. There has
to be a kernel policy involved that a capability can exempt.

> Is this a good enough description ? If not can you clarify your
> expectations ?

The kernel enforces kernel policy. Capabilities provide a mechanism
to mark a process as exempt from some aspect of kernel policy. If
you don't have a kernel policy, you don't get a capability. Clear?

>
>>  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.

I understand pretty darn well.

> 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.

The presumption is that the request is reasonable. Adding a capability
in support of an undefined behavior is unreasonable. Based on the discussion,
CAP_AUDIT_CONTROL is completely rational. I understand that it would be
difficult to support your application privilege model. I would like to look
into helping out with that, but have too many burning knives in the air
just now.

>
> Simo.
>

^ permalink raw reply

* [PATCH 0/4] make function arg and structures as const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall, trond.myklebust, anna.schumaker, bfields, jlayton,
	davem, linux-nfs, linux-kernel, netdev
  Cc: Bhumika Goyal

Make the function argument as const. After thing change, make
the cache_detail structures as const.

Bhumika Goyal (4):
  sunrpc: make the function arg as const
  NFS: make cache_detail structures const
  NFSD: make cache_detail structures const
  SUNRPC: make cache_detail structures const

 fs/nfs/dns_resolve.c              | 2 +-
 fs/nfsd/export.c                  | 4 ++--
 fs/nfsd/nfs4idmap.c               | 4 ++--
 include/linux/sunrpc/cache.h      | 2 +-
 net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
 net/sunrpc/cache.c                | 2 +-
 net/sunrpc/svcauth_unix.c         | 4 ++--
 7 files changed, 11 insertions(+), 11 deletions(-)

-- 
1.9.1

^ permalink raw reply

* [PATCH 1/4] sunrpc: make the function arg as const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall-L2FTfq7BK8M, trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Make the struct cache_detail *tmpl argument of the function
cache_create_net as const as it is only getting passed to kmemup having
the argument as const void *.
Add const to the prototype too.

Signed-off-by: Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 include/linux/sunrpc/cache.h | 2 +-
 net/sunrpc/cache.c           | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/sunrpc/cache.h b/include/linux/sunrpc/cache.h
index 270bad0..40d2822 100644
--- a/include/linux/sunrpc/cache.h
+++ b/include/linux/sunrpc/cache.h
@@ -213,7 +213,7 @@ extern int cache_check(struct cache_detail *detail,
 extern int cache_register_net(struct cache_detail *cd, struct net *net);
 extern void cache_unregister_net(struct cache_detail *cd, struct net *net);
 
-extern struct cache_detail *cache_create_net(struct cache_detail *tmpl, struct net *net);
+extern struct cache_detail *cache_create_net(const struct cache_detail *tmpl, struct net *net);
 extern void cache_destroy_net(struct cache_detail *cd, struct net *net);
 
 extern void sunrpc_init_cache_detail(struct cache_detail *cd);
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 79d55d9..e689438 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1674,7 +1674,7 @@ void cache_unregister_net(struct cache_detail *cd, struct net *net)
 }
 EXPORT_SYMBOL_GPL(cache_unregister_net);
 
-struct cache_detail *cache_create_net(struct cache_detail *tmpl, struct net *net)
+struct cache_detail *cache_create_net(const struct cache_detail *tmpl, struct net *net)
 {
 	struct cache_detail *cd;
 	int i;
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 3/4] NFSD: make cache_detail structures const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall-L2FTfq7BK8M, trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA,
	bfields-uC3wQj2KruNg9hUCZPvPmw, jlayton-vpEMnDpepFuMZCB2o+C8xQ,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

Make these const as they are only getting passed to the function
cache_create_net having the argument as const.

Signed-off-by: Bhumika Goyal <bhumirks-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 fs/nfsd/export.c    | 4 ++--
 fs/nfsd/nfs4idmap.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/export.c b/fs/nfsd/export.c
index 3bc08c3..06bb39c 100644
--- a/fs/nfsd/export.c
+++ b/fs/nfsd/export.c
@@ -231,7 +231,7 @@ static struct cache_head *expkey_alloc(void)
 		return NULL;
 }
 
-static struct cache_detail svc_expkey_cache_template = {
+static const struct cache_detail svc_expkey_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= EXPKEY_HASHMAX,
 	.name		= "nfsd.fh",
@@ -747,7 +747,7 @@ static struct cache_head *svc_export_alloc(void)
 		return NULL;
 }
 
-static struct cache_detail svc_export_cache_template = {
+static const struct cache_detail svc_export_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= EXPORT_HASHMAX,
 	.name		= "nfsd.export",
diff --git a/fs/nfsd/nfs4idmap.c b/fs/nfsd/nfs4idmap.c
index 6b9b6cc..a5bb765 100644
--- a/fs/nfsd/nfs4idmap.c
+++ b/fs/nfsd/nfs4idmap.c
@@ -178,7 +178,7 @@ struct ent {
 static struct ent *idtoname_update(struct cache_detail *, struct ent *,
 				   struct ent *);
 
-static struct cache_detail idtoname_cache_template = {
+static const struct cache_detail idtoname_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= ENT_HASHMAX,
 	.name		= "nfs4.idtoname",
@@ -341,7 +341,7 @@ static struct ent *nametoid_update(struct cache_detail *, struct ent *,
 				   struct ent *);
 static int         nametoid_parse(struct cache_detail *, char *, int);
 
-static struct cache_detail nametoid_cache_template = {
+static const struct cache_detail nametoid_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= ENT_HASHMAX,
 	.name		= "nfs4.nametoid",
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 4/4] SUNRPC: make cache_detail structures const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall, trond.myklebust, anna.schumaker, bfields, jlayton,
	davem, linux-nfs, linux-kernel, netdev
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks@gmail.com>

Make these const as they are only getting passed to the function
cache_create_net having the argument as const.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
 net/sunrpc/svcauth_unix.c         | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 7b1ee5a..e242cb4 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -264,7 +264,7 @@ static int rsi_parse(struct cache_detail *cd,
 	return status;
 }
 
-static struct cache_detail rsi_cache_template = {
+static const struct cache_detail rsi_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= RSI_HASHMAX,
 	.name           = "auth.rpcsec.init",
@@ -524,7 +524,7 @@ static int rsc_parse(struct cache_detail *cd,
 	return status;
 }
 
-static struct cache_detail rsc_cache_template = {
+static const struct cache_detail rsc_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= RSC_HASHMAX,
 	.name		= "auth.rpcsec.context",
diff --git a/net/sunrpc/svcauth_unix.c b/net/sunrpc/svcauth_unix.c
index f81eaa8..740b67d 100644
--- a/net/sunrpc/svcauth_unix.c
+++ b/net/sunrpc/svcauth_unix.c
@@ -569,7 +569,7 @@ static int unix_gid_show(struct seq_file *m,
 	return 0;
 }
 
-static struct cache_detail unix_gid_cache_template = {
+static const struct cache_detail unix_gid_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= GID_HASHMAX,
 	.name		= "auth.unix.gid",
@@ -862,7 +862,7 @@ struct auth_ops svcauth_unix = {
 	.set_client	= svcauth_unix_set_client,
 };
 
-static struct cache_detail ip_map_cache_template = {
+static const struct cache_detail ip_map_cache_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= IP_HASHMAX,
 	.name		= "auth.unix.ip",
-- 
1.9.1

^ permalink raw reply related

* [PATCH 2/4] NFS: make cache_detail structure const
From: Bhumika Goyal @ 2017-10-17 16:14 UTC (permalink / raw)
  To: julia.lawall, trond.myklebust, anna.schumaker, bfields, jlayton,
	davem, linux-nfs, linux-kernel, netdev
  Cc: Bhumika Goyal
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks@gmail.com>

Make it const as it is only getting passed to the function
cache_create_net having the argument as const.

Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
 fs/nfs/dns_resolve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/dns_resolve.c b/fs/nfs/dns_resolve.c
index d25f10f..477934a 100644
--- a/fs/nfs/dns_resolve.c
+++ b/fs/nfs/dns_resolve.c
@@ -353,7 +353,7 @@ ssize_t nfs_dns_resolve_name(struct net *net, char *name,
 	return ret;
 }
 
-static struct cache_detail nfs_dns_resolve_template = {
+static const struct cache_detail nfs_dns_resolve_template = {
 	.owner		= THIS_MODULE,
 	.hash_size	= NFS_DNS_HASHTBL_SIZE,
 	.name		= "dns_resolve",
-- 
1.9.1

^ permalink raw reply related

* Re: [PATCH net] sctp: do not peel off an assoc from one netns to another one
From: Marcelo Ricardo Leitner @ 2017-10-17 16:24 UTC (permalink / raw)
  To: Xin Long; +Cc: network dev, linux-sctp, davem, Neil Horman, chunwang, syzkaller
In-Reply-To: <e637d68ce6f4f94dce8cb30c647e672ebb1f0b7b.1508253970.git.lucien.xin@gmail.com>

On Tue, Oct 17, 2017 at 11:26:10PM +0800, Xin Long wrote:
> 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>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@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
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* Re: [PATCH 44/58] net: ethernet: sun: Convert timers to use timer_setup()
From: Shannon Nelson @ 2017-10-17 16:27 UTC (permalink / raw)
  To: Kees Cook, David S. Miller
  Cc: Philippe Reynes, Jarod Wilson, Rob Herring, chris hyser,
	Tushar Dave, Tobias Klauser, netdev, Thomas Gleixner,
	linux-kernel
In-Reply-To: <1508200182-104605-45-git-send-email-keescook@chromium.org>

On 10/16/2017 5:29 PM, Kees Cook wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly.
> 
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Philippe Reynes <tremyfr@gmail.com>
> Cc: Jarod Wilson <jarod@redhat.com>
> Cc: Shannon Nelson <shannon.nelson@oracle.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: chris hyser <chris.hyser@oracle.com>
> Cc: Tushar Dave <tushar.n.dave@oracle.com>
> Cc: Tobias Klauser <tklauser@distanz.ch>
> Cc: netdev@vger.kernel.org
> Signed-off-by: Kees Cook <keescook@chromium.org>

Acked-by: Shannon Nelson <shannon.nelson@oracle.com>

> ---
>   drivers/net/ethernet/sun/cassini.c        |  7 ++++---
>   drivers/net/ethernet/sun/ldmvsw.c         |  3 +--
>   drivers/net/ethernet/sun/niu.c            | 10 ++++------
>   drivers/net/ethernet/sun/sunbmac.c        | 10 ++++------
>   drivers/net/ethernet/sun/sungem.c         |  6 +++---
>   drivers/net/ethernet/sun/sunhme.c         | 10 ++++------
>   drivers/net/ethernet/sun/sunvnet.c        |  3 +--
>   drivers/net/ethernet/sun/sunvnet_common.c |  4 ++--
>   drivers/net/ethernet/sun/sunvnet_common.h |  2 +-
>   9 files changed, 24 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
> index a74d78f64af9..113bd57e2ea0 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -4079,9 +4079,9 @@ static void cas_reset_task(struct work_struct *work)
>   #endif
>   }
>   
> -static void cas_link_timer(unsigned long data)
> +static void cas_link_timer(struct timer_list *t)
>   {
> -	struct cas *cp = (struct cas *) data;
> +	struct cas *cp = from_timer(cp, t, link_timer);
>   	int mask, pending = 0, reset = 0;
>   	unsigned long flags;
>   
> @@ -5039,7 +5039,8 @@ static int cas_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	spin_lock_init(&cp->stat_lock[N_TX_RINGS]);
>   	mutex_init(&cp->pm_mutex);
>   
> -	setup_timer(&cp->link_timer, cas_link_timer, (unsigned long)cp);
> +	timer_setup(&cp->link_timer, cas_link_timer, 0);
> +
>   #if 1
>   	/* Just in case the implementation of atomic operations
>   	 * change so that an explicit initialization is necessary.
> diff --git a/drivers/net/ethernet/sun/ldmvsw.c b/drivers/net/ethernet/sun/ldmvsw.c
> index 5feeaa9f0a9e..5ea037672e6f 100644
> --- a/drivers/net/ethernet/sun/ldmvsw.c
> +++ b/drivers/net/ethernet/sun/ldmvsw.c
> @@ -363,8 +363,7 @@ static int vsw_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>   	list_add_rcu(&port->list, &vp->port_list);
>   	spin_unlock_irqrestore(&vp->lock, flags);
>   
> -	setup_timer(&port->clean_timer, sunvnet_clean_timer_expire_common,
> -		    (unsigned long)port);
> +	timer_setup(&port->clean_timer, sunvnet_clean_timer_expire_common, 0);
>   
>   	err = register_netdev(dev);
>   	if (err) {
> diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
> index bde19b307d0d..ab502ee35fb2 100644
> --- a/drivers/net/ethernet/sun/niu.c
> +++ b/drivers/net/ethernet/sun/niu.c
> @@ -2221,9 +2221,9 @@ static int niu_link_status(struct niu *np, int *link_up_p)
>   	return err;
>   }
>   
> -static void niu_timer(unsigned long __opaque)
> +static void niu_timer(struct timer_list *t)
>   {
> -	struct niu *np = (struct niu *) __opaque;
> +	struct niu *np = from_timer(np, t, timer);
>   	unsigned long off;
>   	int err, link_up;
>   
> @@ -6123,7 +6123,7 @@ static int niu_open(struct net_device *dev)
>   
>   	err = niu_init_hw(np);
>   	if (!err) {
> -		setup_timer(&np->timer, niu_timer, (unsigned long)np);
> +		timer_setup(&np->timer, niu_timer, 0);
>   		np->timer.expires = jiffies + HZ;
>   
>   		err = niu_enable_interrupts(np, 1);
> @@ -6773,10 +6773,8 @@ static int niu_change_mtu(struct net_device *dev, int new_mtu)
>   
>   	err = niu_init_hw(np);
>   	if (!err) {
> -		init_timer(&np->timer);
> +		timer_setup(&np->timer, niu_timer, 0);
>   		np->timer.expires = jiffies + HZ;
> -		np->timer.data = (unsigned long) np;
> -		np->timer.function = niu_timer;
>   
>   		err = niu_enable_interrupts(np, 1);
>   		if (err)
> diff --git a/drivers/net/ethernet/sun/sunbmac.c b/drivers/net/ethernet/sun/sunbmac.c
> index 3189722110c2..0b1f41f6bceb 100644
> --- a/drivers/net/ethernet/sun/sunbmac.c
> +++ b/drivers/net/ethernet/sun/sunbmac.c
> @@ -523,9 +523,9 @@ static int try_next_permutation(struct bigmac *bp, void __iomem *tregs)
>   	return -1;
>   }
>   
> -static void bigmac_timer(unsigned long data)
> +static void bigmac_timer(struct timer_list *t)
>   {
> -	struct bigmac *bp = (struct bigmac *) data;
> +	struct bigmac *bp = from_timer(bp, t, bigmac_timer);
>   	void __iomem *tregs = bp->tregs;
>   	int restart_timer = 0;
>   
> @@ -613,8 +613,6 @@ static void bigmac_begin_auto_negotiation(struct bigmac *bp)
>   	bp->timer_state = ltrywait;
>   	bp->timer_ticks = 0;
>   	bp->bigmac_timer.expires = jiffies + (12 * HZ) / 10;
> -	bp->bigmac_timer.data = (unsigned long) bp;
> -	bp->bigmac_timer.function = bigmac_timer;
>   	add_timer(&bp->bigmac_timer);
>   }
>   
> @@ -921,7 +919,7 @@ static int bigmac_open(struct net_device *dev)
>   		printk(KERN_ERR "BIGMAC: Can't order irq %d to go.\n", dev->irq);
>   		return ret;
>   	}
> -	init_timer(&bp->bigmac_timer);
> +	timer_setup(&bp->bigmac_timer, bigmac_timer, 0);
>   	ret = bigmac_init_hw(bp, 0);
>   	if (ret)
>   		free_irq(dev->irq, bp);
> @@ -1172,7 +1170,7 @@ static int bigmac_ether_init(struct platform_device *op,
>   					      "board-version", 1);
>   
>   	/* Init auto-negotiation timer state. */
> -	init_timer(&bp->bigmac_timer);
> +	timer_setup(&bp->bigmac_timer, bigmac_timer, 0);
>   	bp->timer_state = asleep;
>   	bp->timer_ticks = 0;
>   
> diff --git a/drivers/net/ethernet/sun/sungem.c b/drivers/net/ethernet/sun/sungem.c
> index b75ab8f44968..a7afcee3c5ae 100644
> --- a/drivers/net/ethernet/sun/sungem.c
> +++ b/drivers/net/ethernet/sun/sungem.c
> @@ -1496,9 +1496,9 @@ static int gem_mdio_link_not_up(struct gem *gp)
>   	}
>   }
>   
> -static void gem_link_timer(unsigned long data)
> +static void gem_link_timer(struct timer_list *t)
>   {
> -	struct gem *gp = (struct gem *) data;
> +	struct gem *gp = from_timer(gp, t, link_timer);
>   	struct net_device *dev = gp->dev;
>   	int restart_aneg = 0;
>   
> @@ -2910,7 +2910,7 @@ static int gem_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>   
>   	gp->msg_enable = DEFAULT_MSG;
>   
> -	setup_timer(&gp->link_timer, gem_link_timer, (unsigned long)gp);
> +	timer_setup(&gp->link_timer, gem_link_timer, 0);
>   
>   	INIT_WORK(&gp->reset_task, gem_reset_task);
>   
> diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
> index 9e983e1d8249..0431f1e5f511 100644
> --- a/drivers/net/ethernet/sun/sunhme.c
> +++ b/drivers/net/ethernet/sun/sunhme.c
> @@ -685,9 +685,9 @@ static int is_lucent_phy(struct happy_meal *hp)
>   	return ret;
>   }
>   
> -static void happy_meal_timer(unsigned long data)
> +static void happy_meal_timer(struct timer_list *t)
>   {
> -	struct happy_meal *hp = (struct happy_meal *) data;
> +	struct happy_meal *hp = from_timer(hp, t, happy_timer);
>   	void __iomem *tregs = hp->tcvregs;
>   	int restart_timer = 0;
>   
> @@ -1413,8 +1413,6 @@ happy_meal_begin_auto_negotiation(struct happy_meal *hp,
>   
>   	hp->timer_ticks = 0;
>   	hp->happy_timer.expires = jiffies + (12 * HZ)/10;  /* 1.2 sec. */
> -	hp->happy_timer.data = (unsigned long) hp;
> -	hp->happy_timer.function = happy_meal_timer;
>   	add_timer(&hp->happy_timer);
>   }
>   
> @@ -2819,7 +2817,7 @@ static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe)
>   	hp->timer_state = asleep;
>   	hp->timer_ticks = 0;
>   
> -	init_timer(&hp->happy_timer);
> +	timer_setup(&hp->happy_timer, happy_meal_timer, 0);
>   
>   	hp->dev = dev;
>   	dev->netdev_ops = &hme_netdev_ops;
> @@ -3133,7 +3131,7 @@ static int happy_meal_pci_probe(struct pci_dev *pdev,
>   	hp->timer_state = asleep;
>   	hp->timer_ticks = 0;
>   
> -	init_timer(&hp->happy_timer);
> +	timer_setup(&hp->happy_timer, happy_meal_timer, 0);
>   
>   	hp->irq = pdev->irq;
>   	hp->dev = dev;
> diff --git a/drivers/net/ethernet/sun/sunvnet.c b/drivers/net/ethernet/sun/sunvnet.c
> index 0b95105f7060..27fb22638885 100644
> --- a/drivers/net/ethernet/sun/sunvnet.c
> +++ b/drivers/net/ethernet/sun/sunvnet.c
> @@ -492,8 +492,7 @@ static int vnet_port_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>   	pr_info("%s: PORT ( remote-mac %pM%s )\n",
>   		vp->dev->name, port->raddr, switch_port ? " switch-port" : "");
>   
> -	setup_timer(&port->clean_timer, sunvnet_clean_timer_expire_common,
> -		    (unsigned long)port);
> +	timer_setup(&port->clean_timer, sunvnet_clean_timer_expire_common, 0);
>   
>   	napi_enable(&port->napi);
>   	vio_port_up(&port->vio);
> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
> index ecf456c7b6d1..8aa3ce46bb81 100644
> --- a/drivers/net/ethernet/sun/sunvnet_common.c
> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
> @@ -1040,9 +1040,9 @@ static inline void vnet_free_skbs(struct sk_buff *skb)
>   	}
>   }
>   
> -void sunvnet_clean_timer_expire_common(unsigned long port0)
> +void sunvnet_clean_timer_expire_common(struct timer_list *t)
>   {
> -	struct vnet_port *port = (struct vnet_port *)port0;
> +	struct vnet_port *port = from_timer(port, t, clean_timer);
>   	struct sk_buff *freeskbs;
>   	unsigned pending;
>   
> diff --git a/drivers/net/ethernet/sun/sunvnet_common.h b/drivers/net/ethernet/sun/sunvnet_common.h
> index b20d6fa7ef25..656673c31066 100644
> --- a/drivers/net/ethernet/sun/sunvnet_common.h
> +++ b/drivers/net/ethernet/sun/sunvnet_common.h
> @@ -129,7 +129,7 @@ struct vnet {
>   	((__port)->vsw ? (__port)->dev : (__port)->vp->dev)
>   
>   /* Common funcs */
> -void sunvnet_clean_timer_expire_common(unsigned long port0);
> +void sunvnet_clean_timer_expire_common(struct timer_list *t);
>   int sunvnet_open_common(struct net_device *dev);
>   int sunvnet_close_common(struct net_device *dev);
>   void sunvnet_set_rx_mode_common(struct net_device *dev, struct vnet *vp);
> 

^ permalink raw reply

* Re: [RFC] sctp: suspicious rcu_read_lock() in sctp_packet_config()
From: Marcelo Ricardo Leitner @ 2017-10-17 16:28 UTC (permalink / raw)
  To: Xin Long
  Cc: Eric Dumazet, Vlad Yasevich, Neil Horman, netdev, Wei Wang,
	Eric Dumazet, linux-sctp
In-Reply-To: <CADvbK_dRBaOLzrXWS2ndHu3sz9LB87rJsyGog2BjishapiMhxw@mail.gmail.com>

On Tue, Oct 17, 2017 at 11:31:30PM +0800, Xin Long wrote:
> 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.

I checked some and sctp_transport_dst_check() should be fine because
by then we are holding a reference on dst. Same goes to
sctp_transport_pmtu_check().  It's not possible that these would trip
on the update going on on sctp_packet_config() because the socket is
locked. We may not need (much) more than the example patch, I think.

A more thorough check is certainly welcomed, indeed.

  Marcelo

^ permalink raw reply

* Re: [PATCH net 2/3] bpf: fix splat for illegal devmap percpu allocation
From: John Fastabend @ 2017-10-17 16:28 UTC (permalink / raw)
  To: Daniel Borkmann, davem
  Cc: tj, ast, mark.rutland, richard, sp3485, netdev, linux-kernel
In-Reply-To: <dd091b1fec763d74185a156ecd2f972810bdd1ce.1508251210.git.daniel@iogearbox.net>

On 10/17/2017 07:55 AM, Daniel Borkmann wrote:
> 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>
> ---

Thanks!

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply

* Re: [PATCH net 3/3] bpf: do not test for PCPU_MIN_UNIT_SIZE before percpu allocations
From: John Fastabend @ 2017-10-17 16:29 UTC (permalink / raw)
  To: Daniel Borkmann, davem
  Cc: tj, ast, mark.rutland, richard, sp3485, netdev, linux-kernel
In-Reply-To: <a11e92b8ba4f5270437caa629307c300bca37976.1508251210.git.daniel@iogearbox.net>

On 10/17/2017 07:55 AM, Daniel Borkmann wrote:
> 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>
> ---

Nice cleanup. Thanks!

Acked-by: John Fastabend <john.fastabend@gmail.com>

^ permalink raw reply

* Re: [PATCH 0/4] make function arg and structures as const
From: Jeff Layton @ 2017-10-17 16:40 UTC (permalink / raw)
  To: Bhumika Goyal, julia.lawall, trond.myklebust, anna.schumaker,
	bfields, davem, linux-nfs, linux-kernel, netdev
In-Reply-To: <1508256866-12798-1-git-send-email-bhumirks@gmail.com>

On Tue, 2017-10-17 at 18:14 +0200, Bhumika Goyal wrote:
> Make the function argument as const. After thing change, make
> the cache_detail structures as const.
> 
> Bhumika Goyal (4):
>   sunrpc: make the function arg as const
>   NFS: make cache_detail structures const
>   NFSD: make cache_detail structures const
>   SUNRPC: make cache_detail structures const
> 
>  fs/nfs/dns_resolve.c              | 2 +-
>  fs/nfsd/export.c                  | 4 ++--
>  fs/nfsd/nfs4idmap.c               | 4 ++--
>  include/linux/sunrpc/cache.h      | 2 +-
>  net/sunrpc/auth_gss/svcauth_gss.c | 4 ++--
>  net/sunrpc/cache.c                | 2 +-
>  net/sunrpc/svcauth_unix.c         | 4 ++--
>  7 files changed, 11 insertions(+), 11 deletions(-)
> 

Looks pretty straightforward. You can add this to the set:

Reviewed-by: Jeff Layton <jlayton@redhat.com>

^ permalink raw reply

* Re: RFC(v2): Audit Kernel Container IDs
From: Casey Schaufler @ 2017-10-17 16:43 UTC (permalink / raw)
  To: James Bottomley, Simo Sorce, Steve Grubb, linux-audit
  Cc: mszeredi, trondmy, jlayton, Linux API, Linux Containers,
	Linux Kernel, David Howells, Carlos O'Donell, cgroups,
	Eric W. Biederman, Andy Lutomirski, Linux Network Development,
	Linux FS Devel, Eric Paris, Al Viro
In-Reply-To: <1508255091.3129.27.camel@HansenPartnership.com>

On 10/17/2017 8:44 AM, James Bottomley wrote:
> 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.

That would be PTAGS. I agree that such a general mechanism
could be very useful for a variety of purposes, not just
containers. I do not agree that a single integer (e.g. a
containerID) warrants more than trivial mechanism.

> The label should only be susceptible
> to modification by something possessing a capability (which one TBD).

I think that the reason we're going to have crying and gnashing
of teeth is that whatever capability is used. There will always be
an issue of the capability granted being less specific than the
application security model would like.

And no, we're not going down the 330 capabilities road. It's been
done in the UNIX world. Application security models hate that
just as much as they hate the coarser granularity.

> 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.

I'm fine with that. The user space policy can be anything y'all like.

> The label will be used as a tag for audit information.

Deep breath ...

Which *is* a kernel security policy mechanism. Since the "label"
is part of the audit information that the kernel is guaranteeing
changing it would be covered by CAP_AUDIT_CONTROL. If the kernel
does not use the "label" for any other purpose this is the only
capability that makes sense for it.

> 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.  

Yes. This is a consequence of the capability granularity. There is
no way we can make the capability granularity sufficiently fine to
prevent this. No one wants the 330 capabilities that Data General
had in their secure UNIX system. 

> 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.

You can't support this reasonably with a single containerID.
You want PTAGS for this. I know that there is resistance to
requiring anything beyond what's in the base kernel (and for
good reasons) for containers. Especially something that is
pending future work. But let's not jam something into the base
kernel that isn't really going to address the issue.

> James

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox