Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Alexei Starovoitov @ 2018-09-20  0:59 UTC (permalink / raw)
  To: Song Liu, Alexei Starovoitov
  Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
	acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <3FB6EC86-700C-4C80-A467-55B6753B07FE@fb.com>

On 9/19/18 4:44 PM, Song Liu wrote:
>
>
>> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>>
>> use perf_event_mmap_bpf_prog() helper to notify user space
>> about JITed bpf programs.
>> Use RECORD_MMAP perf event to tell user space where JITed bpf program was loaded.
>> Use empty program name as unload indication.
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> ---
>> kernel/bpf/core.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 3f5bf1af0826..ddf11fdafd36 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -384,7 +384,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
>> 	*symbol_end   = addr + hdr->pages * PAGE_SIZE;
>> }
>>
>> -static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> +static char *bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> {
>> 	const char *end = sym + KSYM_NAME_LEN;
>>
>> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> 	sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
>> 	sym  = bin2hex(sym, prog->tag, sizeof(prog->tag));
>> 	if (prog->aux->name[0])
>> -		snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
>> +		sym += snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
>> 	else
>> 		*sym = 0;
>> +	return sym;
>> }
>>
>> static __always_inline unsigned long
>> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
>>
>> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
>> {
>> +	unsigned long symbol_start, symbol_end;
>> +	char buf[KSYM_NAME_LEN], *sym;
>> +
>> 	if (!bpf_prog_kallsyms_candidate(fp) ||
>> 	    !capable(CAP_SYS_ADMIN))
>> 		return;
>>
>> +	bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
>> +	sym = bpf_get_prog_name(fp, buf);
>> +	sym++; /* sym - buf is the length of the name including trailing 0 */
>> +	while (!IS_ALIGNED(sym - buf, sizeof(u64)))
>> +		*sym++ = 0;
>
> nit: This logic feels a little weird to me. How about we wrap the extra logic
> in a separate function:
>
> size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf)

probably bpf_get_prog_name_u64_padded() ?
would be cleaner indeed.

Will send v2 once Peter and Arnaldo provide their feedback.

Thanks for reviewing!

^ permalink raw reply

* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Wang, Dongsheng @ 2018-09-20  1:33 UTC (permalink / raw)
  To: Timur Tabi, Andrew Lunn
  Cc: Florian Fainelli, davem@davemloft.net, Zheng, Joey,
	netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <1226d3d3-e2ba-571d-5b70-b246cbde2fb6@kernel.org>

On 2018/9/19 22:15, Timur Tabi wrote:
> On 9/19/18 7:25 AM, Andrew Lunn wrote:
>> ACPI is completely separate and should not affect the DT binding.
>> I've not yet looked at the ACPI changes you added.
> Just FYI, there is no device tree platform on which the upstream EMAC 
> driver works.  All of the DT code in the driver is theoretical.  It 
> worked once on a prototype platform, when I originally wrote the code, 
> but since then DT support is mostly a guess.
>
> The focus of any patches for the EMAC should be ACPI, not DT.  If 
> anything, ACPI support should come first.  No one should be writing or 
> reviewing DT code before ACPI code.
>
> The upstream EMAC driver is only known to work on the QDF2400, which is 
> an ACPI-only chip.  I feel like I've been repeating this too often.
>
Ok, I just focus on ACPI, and keep DT code no changes.

Cheers,
Dongsheng

^ permalink raw reply

* Re: [PATCH] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-20  7:18 UTC (permalink / raw)
  To: casey
  Cc: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <75b3fed9-e549-4ed0-c435-ec4795fc1e39@schaufler-ca.com>

> I've only had a cursory look at your patch, but how is it
> different from what's in xt_SECMARK.c ?

xt_SEXMARK.c is for xtables, use-able in iptables; this is for nftables (nft)

^ permalink raw reply

* [PATCH] net: netronome: remove redundant continue
From: zhong jiang @ 2018-09-20  8:02 UTC (permalink / raw)
  To: davem; +Cc: dirk.vandermerwe, simon.horman, netdev, linux-kernel

The continue will not truely skip any code. hence it is safe to
remove it.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/netronome/nfp/nfp_net_main.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
index 0b1ac9c..50d7b58 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_net_main.c
@@ -230,10 +230,8 @@ static void nfp_net_pf_free_vnics(struct nfp_pf *pf)
 		ctrl_bar += NFP_PF_CSR_SLICE_SIZE;
 
 		/* Kill the vNIC if app init marked it as invalid */
-		if (nn->port && nn->port->type == NFP_PORT_INVALID) {
+		if (nn->port && nn->port->type == NFP_PORT_INVALID)
 			nfp_net_pf_free_vnic(pf, nn);
-			continue;
-		}
 	}
 
 	if (list_empty(&pf->vnics))
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH 00/22] various dynamic_debug patches
From: Rafael J. Wysocki @ 2018-09-20  8:05 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Jason Baron, Andrew Morton, linux-kernel, Greg Kroah-Hartman,
	netdev, Petr Mladek, Steven Rostedt, linux-btrfs, linux-acpi, x86
In-Reply-To: <20180919220444.23190-1-linux@rasmusvillemoes.dk>

On Thursday, September 20, 2018 12:04:22 AM CEST Rasmus Villemoes wrote:
> This started as an experiment to see how hard it would be to change
> the four pointers in struct _ddebug into relative offsets, a la
> CONFIG_GENERIC_BUG_RELATIVE_POINTERS, thus saving 16 bytes per
> pr_debug site (and thus exactly making up for the extra space used by
> the introduction of jump labels in 9049fc74). I stumbled on a few
> things that are probably worth fixing regardless of whether the latter
> half of this series is deemed worthwhile.
> 
> Patch relationships: 1-2, 3-4, 5-6 and 15-16 can be applied
> individually, though 2, 4 and 6 probably makes most sense in the
> context of the final goal of the series.

OK, I can take the [15-16/22] separately.

Thanks,
Rafael

^ permalink raw reply

* [PATCH] net: amd: remove redundant continue
From: zhong jiang @ 2018-09-20  8:07 UTC (permalink / raw)
  To: davem; +Cc: netdev, linux-kernel

The continue will not truely skip any code. hence it is safe to
remove it.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/amd/ni65.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/amd/ni65.c b/drivers/net/ethernet/amd/ni65.c
index e248d1a..8931ce6 100644
--- a/drivers/net/ethernet/amd/ni65.c
+++ b/drivers/net/ethernet/amd/ni65.c
@@ -435,10 +435,8 @@ static int __init ni65_probe1(struct net_device *dev,int ioaddr)
 		}
 		if(cards[i].vendor_id) {
 			for(j=0;j<3;j++)
-				if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j]) {
+				if(inb(ioaddr+cards[i].addr_offset+j) != cards[i].vendor_id[j])
 					release_region(ioaddr, cards[i].total_size);
-					continue;
-			  }
 		}
 		break;
 	}
-- 
1.7.12.4

^ permalink raw reply related

* Re: [PATCH 00/11] pull request for net: batman-adv 2018-09-19
From: Simon Wunderlich @ 2018-09-20  8:09 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	b.a.t.m.a.n-ZwoEplunGu2X36UT3dwllkB+6BGkLq7r
In-Reply-To: <20180919.203434.1784595444537176012.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

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

On Wednesday, September 19, 2018 8:34:34 PM CEST David Miller wrote:
> From: Simon Wunderlich <sw-2YrNx6rUIHYiY0qSoAWiAoQuADTiUCJX@public.gmane.org>
> Date: Wed, 19 Sep 2018 14:22:02 +0200
> 
> > here are some bugfixes which we would like to see integrated into net.
> > 
> > We forgot to bump the version number in the last round for net-next, so
> > the belated patch to do that is included - we hope you can adopt it.
> > This will most likely create a merge conflict later when merging into
> > net-next with this rounds net-next patchset, but net-next should keep
> > the 2018.4 version[1].
> > 
> > Please pull or let me know of any problem!
> 
> Pulled, thanks Simon.

Great, thanks!

> 
> In the future, you can put that merge resolution guidance into the
> merge commit message.
> 
> When people give annotations like this, I refer to them when I do
> merges and it helps me a lot.  If it's in the actual GIT history,
> that is tons easier than if it just appeared in your pull request
> email.
> 
> I manually included it in the merge commit message this time.
> 
> Thanks!

OK, thanks for the hint! We will do this.

Cheers,
     Simon

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* [PATCH net-next] atl1: remove set but not used variable 'advertising'
From: YueHaibing @ 2018-09-20  2:41 UTC (permalink / raw)
  To: Jay Cliburn, Chris Snook, David S. Miller, Kees Cook, zhong jiang
  Cc: YueHaibing, netdev, kernel-janitors, linux-kernel

Fixes gcc '-Wunused-but-set-variable' warning:

drivers/net/ethernet/atheros/atlx/atl1.c: In function 'atl1_set_link_ksettings':
drivers/net/ethernet/atheros/atlx/atl1.c:3280:6: warning:
 variable 'advertising' set but not used [-Wunused-but-set-variable]

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 drivers/net/ethernet/atheros/atlx/atl1.c | 21 +--------------------
 1 file changed, 1 insertion(+), 20 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl1.c b/drivers/net/ethernet/atheros/atlx/atl1.c
index 72e7fa7..63edc57 100644
--- a/drivers/net/ethernet/atheros/atlx/atl1.c
+++ b/drivers/net/ethernet/atheros/atlx/atl1.c
@@ -3277,7 +3277,6 @@ static int atl1_set_link_ksettings(struct net_device *netdev,
 	u16 phy_data;
 	int ret_val = 0;
 	u16 old_media_type = hw->media_type;
-	u32 advertising;
 
 	if (netif_running(adapter->netdev)) {
 		if (netif_msg_link(adapter))
@@ -3311,25 +3310,7 @@ static int atl1_set_link_ksettings(struct net_device *netdev,
 				hw->media_type = MEDIA_TYPE_10M_HALF;
 		}
 	}
-	switch (hw->media_type) {
-	case MEDIA_TYPE_AUTO_SENSOR:
-		advertising =
-		    ADVERTISED_10baseT_Half |
-		    ADVERTISED_10baseT_Full |
-		    ADVERTISED_100baseT_Half |
-		    ADVERTISED_100baseT_Full |
-		    ADVERTISED_1000baseT_Full |
-		    ADVERTISED_Autoneg | ADVERTISED_TP;
-		break;
-	case MEDIA_TYPE_1000M_FULL:
-		advertising =
-		    ADVERTISED_1000baseT_Full |
-		    ADVERTISED_Autoneg | ADVERTISED_TP;
-		break;
-	default:
-		advertising = 0;
-		break;
-	}
+
 	if (atl1_phy_setup_autoneg_adv(hw)) {
 		ret_val = -EINVAL;
 		if (netif_msg_link(adapter))

^ permalink raw reply related

* [PATCH] net: neterion: remove redundant continue
From: zhong jiang @ 2018-09-20  8:13 UTC (permalink / raw)
  To: davem; +Cc: jdmason, netdev, linux-kernel

The continue will not truely skip any code. hence it is safe to
remove it.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/neterion/s2io.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/neterion/s2io.c b/drivers/net/ethernet/neterion/s2io.c
index f980f10..3c2ac6af 100644
--- a/drivers/net/ethernet/neterion/s2io.c
+++ b/drivers/net/ethernet/neterion/s2io.c
@@ -3679,11 +3679,9 @@ static void restore_xmsi_data(struct s2io_nic *nic)
 		writeq(nic->msix_info[i].data, &bar0->xmsi_data);
 		val64 = (s2BIT(7) | s2BIT(15) | vBIT(msix_index, 26, 6));
 		writeq(val64, &bar0->xmsi_access);
-		if (wait_for_msix_trans(nic, msix_index)) {
+		if (wait_for_msix_trans(nic, msix_index))
 			DBG_PRINT(ERR_DBG, "%s: index: %d failed\n",
 				  __func__, msix_index);
-			continue;
-		}
 	}
 }
 
-- 
1.7.12.4

^ permalink raw reply related

* Re: [RFC bpf-next 2/4] bpf: return EOPNOTSUPP when map lookup isn't supported
From: Prashant Bhole @ 2018-09-20  2:34 UTC (permalink / raw)
  To: Mauricio Vasquez, Alexei Starovoitov
  Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
	Quentin Monnet, David S . Miller, netdev
In-Reply-To: <bd05a444-95b0-8573-4bbc-4ce0a10f7042@polito.it>



On 9/20/2018 3:40 AM, Mauricio Vasquez wrote:
> 
> 
> On 09/19/2018 10:14 AM, Alexei Starovoitov wrote:
>> On Wed, Sep 19, 2018 at 04:51:41PM +0900, Prashant Bhole wrote:
>>> Return ERR_PTR(-EOPNOTSUPP) from map_lookup_elem() methods of below
>>> map types:
>>> - BPF_MAP_TYPE_PROG_ARRAY
>>> - BPF_MAP_TYPE_STACK_TRACE
>>> - BPF_MAP_TYPE_XSKMAP
>>> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
>>>
>>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>>> ---
>>>   kernel/bpf/arraymap.c | 2 +-
>>>   kernel/bpf/sockmap.c  | 2 +-
>>>   kernel/bpf/stackmap.c | 2 +-
>>>   kernel/bpf/xskmap.c   | 2 +-
>>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>>> index dded84cbe814..24583da9ffd1 100644
>>> --- a/kernel/bpf/arraymap.c
>>> +++ b/kernel/bpf/arraymap.c
>>> @@ -449,7 +449,7 @@ static void fd_array_map_free(struct bpf_map *map)
>>>   static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
>>>   {
>>> -    return NULL;
>>> +    return ERR_PTR(-EOPNOTSUPP);
>>>   }
>> conceptually the set looks good to me.
>> Please add a test to test_verifier.c to make sure
>> that these lookup helpers cannot be called from BPF program.
>> Otherwise this diff may cause crashes.
>>
>>
> I think we can remove all these stub functions that return EOPNOTSUPP or 
> EINVAL and return the error in syscall.c if ops->map_[update, delete, 
> lookup, ...] is null.
> This will require to extend (and test) the verifier to guarantee that 
> those helpers are not called in wrong maps, for example 
> map_delete_element in array like maps.
> 
> Would it make sense?

Thanks for review and suggestion.

I had thought about this way too (except adding restrictions in the 
verifier). There is no strong reason for choosing current implementation.

I thought there must be some reason that those methods are implemented 
and just returning NULL. Also there are no NULL checks for 
map_lookup_elem stub. So I decided to simply change the return value.
If some more people agree with removing stub function idea, I will do it.

-Prashant

^ permalink raw reply

* Re: [RFC bpf-next 2/4] bpf: return EOPNOTSUPP when map lookup isn't supported
From: Prashant Bhole @ 2018-09-20  2:44 UTC (permalink / raw)
  To: Alexei Starovoitov
  Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
	Quentin Monnet, David S . Miller, netdev
In-Reply-To: <20180919151422.tysdcku2hxaq37xw@ast-mbp>



On 9/20/2018 12:14 AM, Alexei Starovoitov wrote:
> On Wed, Sep 19, 2018 at 04:51:41PM +0900, Prashant Bhole wrote:
>> Return ERR_PTR(-EOPNOTSUPP) from map_lookup_elem() methods of below
>> map types:
>> - BPF_MAP_TYPE_PROG_ARRAY
>> - BPF_MAP_TYPE_STACK_TRACE
>> - BPF_MAP_TYPE_XSKMAP
>> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
>>
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>>   kernel/bpf/arraymap.c | 2 +-
>>   kernel/bpf/sockmap.c  | 2 +-
>>   kernel/bpf/stackmap.c | 2 +-
>>   kernel/bpf/xskmap.c   | 2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>> index dded84cbe814..24583da9ffd1 100644
>> --- a/kernel/bpf/arraymap.c
>> +++ b/kernel/bpf/arraymap.c
>> @@ -449,7 +449,7 @@ static void fd_array_map_free(struct bpf_map *map)
>>   
>>   static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
>>   {
>> -	return NULL;
>> +	return ERR_PTR(-EOPNOTSUPP);
>>   }
> 
> conceptually the set looks good to me.
> Please add a test to test_verifier.c to make sure
> that these lookup helpers cannot be called from BPF program.
> Otherwise this diff may cause crashes.

Thanks for reviewing.
Is the verifier change below sufficient?

--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2128,10 +2128,18 @@ static int check_map_func_compatibility(struct 
bpf_verifier_env *env,
  		if (env->subprog_cnt > 1) {
  			verbose(env, "tail_calls are not allowed in programs with 
bpf-to-bpf calls\n");
  			return -EINVAL;
  		}
  		break;
+	case BPF_FUNC_map_lookup_elem:
+		if (map->map_type == BPF_MAP_TYPE_PROG_ARRAY ||
+		    map->map_type == BPF_MAP_TYPE_STACK_TRACE ||
+		    map->map_type == BPF_MAP_TYPE_XSKMAP ||
+		    map->map_type == BPF_MAP_TYPE_SOCKMAP ||
+		    map->map_type == BPF_MAP_TYPE_SOCKHASH)
+			goto error;
+		break;
  	case BPF_FUNC_perf_event_read:
  	case BPF_FUNC_perf_event_output:
  	case BPF_FUNC_perf_event_read_value:
  		if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
  			goto error;

-Prashant

^ permalink raw reply

* Re: [RFC bpf-next 3/4] tools/bpf: bpftool, split the function do_dump()
From: Prashant Bhole @ 2018-09-20  2:49 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
	David S . Miller, netdev
In-Reply-To: <20180919082617.0967a1cf@cakuba.netronome.com>



On 9/20/2018 12:26 AM, Jakub Kicinski wrote:
> On Wed, 19 Sep 2018 16:51:42 +0900, Prashant Bhole wrote:
>> +static int dump_map_elem(int fd, void *key, void *value,
>> +			 struct bpf_map_info *map_info, struct btf *btf,
>> +			 json_writer_t *btf_wtr)
>> +{
>> +	int num_elems = 0;
>> +
>> +	if (!bpf_map_lookup_elem(fd, key, value)) {
>> +		if (json_output) {
>> +			print_entry_json(map_info, key, value, btf);
>> +		} else {
>> +			if (btf) {
>> +				struct btf_dumper d = {
>> +					.btf = btf,
>> +					.jw = btf_wtr,
>> +					.is_plain_text = true,
>> +				};
>> +
>> +				do_dump_btf(&d, map_info, key, value);
>> +			} else {
>> +				print_entry_plain(map_info, key, value);
>> +			}
>> +			num_elems++;
>> +		}
>> +		goto out;
>> +	}
>> +
>> +	/* lookup error handling */
>> +	if (map_is_map_of_maps(map_info->type) ||
>> +	    map_is_map_of_progs(map_info->type))
>> +		goto out;
>> +
> 
> nit: why not just return?  the goto seems to only do a return anyway,
>       is this suggested by some coding style?  Is it to help the
>       compiler?  I see people do this from time to time..

Thanks for reviewing. I agree, goto and the label isn't needed. I will 
fix it.

-Prashant
> 
> [...]
> 
>> +out:
>> +	return num_elems;
> 
> 

^ permalink raw reply

* Re: [RFC bpf-next 4/4] tools/bpf: handle EOPNOTSUPP when map lookup is failed
From: Prashant Bhole @ 2018-09-20  2:58 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Alexei Starovoitov, Daniel Borkmann, Quentin Monnet,
	David S . Miller, netdev
In-Reply-To: <20180919082954.50a827a0@cakuba.netronome.com>



On 9/20/2018 12:29 AM, Jakub Kicinski wrote:
> On Wed, 19 Sep 2018 16:51:43 +0900, Prashant Bhole wrote:
>> Let's add a check for EOPNOTSUPP error when map lookup is failed.
>> Also in case map doesn't support lookup, the output of map dump is
>> changed from "can't lookup element" to "lookup not supported for
>> this map".
>>
>> Patch adds function print_entry_error() function to print the error
>> value.
>>
>> Following example dumps a map which does not support lookup.
>>
>> Output before:
>> root# bpftool map -jp dump id 40
>> [
>>      "key": ["0x0a","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "can\'t lookup element"
>>      },
>>      "key": ["0x0b","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "can\'t lookup element"
>>      }
>> ]
>>
>> root# bpftool map dump id 40
>> can't lookup element with key:
>> 0a 00 00 00
>> can't lookup element with key:
>> 0b 00 00 00
>> Found 0 elements
>>
>> Output after changes:
>> root# bpftool map dump -jp  id 45
>> [
>>      "key": ["0x0a","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "lookup not supported for this map"
>>      },
>>      "key": ["0x0b","0x00","0x00","0x00"
>>      ],
>>      "value": {
>>          "error": "lookup not supported for this map"
>>      }
>> ]
>>
>> root# bpftool map dump id 45
>> key:
>> 0a 00 00 00
>> value:
>> lookup not supported for this map
>> key:
>> 0b 00 00 00
>> value:
>> lookup not supported for this map
>> Found 0 elements
> 
> Nice improvement, thanks for the changes!  I wonder what your thoughts
> would be on just printing some form of "lookup not supported for this
> map" only once?  It seems slightly like repeated information - if
> lookup is not supported for one key it likely won't be for other keys
> too, so we could shorten the output.  Would that make sense?

I too thought that the message is repeated information. One idea was as 
you said, stop iterating once we get EOPNOTSUPP. But only reason for 
keeping this way is that user will at least see dump of keys along with 
it. Also it will be consistent with Json output. What is your opinion on it?

I will fix other things that you pointed out. Thanks!

-Prashant

> 
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>>   tools/bpf/bpftool/main.h |  5 +++++
>>   tools/bpf/bpftool/map.c  | 35 ++++++++++++++++++++++++++++++-----
>>   2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/bpf/bpftool/main.h b/tools/bpf/bpftool/main.h
>> index 40492cdc4e53..1a8c683f949b 100644
>> --- a/tools/bpf/bpftool/main.h
>> +++ b/tools/bpf/bpftool/main.h
>> @@ -46,6 +46,11 @@
>>   
>>   #include "json_writer.h"
>>   
>> +#define ERR_CANNOT_LOOKUP \
>> +	"can't lookup element"
>> +#define ERR_LOOKUP_NOT_SUPPORTED \
>> +	"lookup not supported for this map"
> 
> Do we need these?  Are we going to reused them in more parts of the
> code?
> 
>>   #define ptr_to_u64(ptr)	((__u64)(unsigned long)(ptr))
>>   
>>   #define NEXT_ARG()	({ argc--; argv++; if (argc < 0) usage(); })
>> diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
>> index 284e12a289c0..2faccd2098c9 100644
>> --- a/tools/bpf/bpftool/map.c
>> +++ b/tools/bpf/bpftool/map.c
>> @@ -333,6 +333,25 @@ static void print_entry_json(struct bpf_map_info *info, unsigned char *key,
>>   	jsonw_end_object(json_wtr);
>>   }
>>   
>> +static void print_entry_error(struct bpf_map_info *info, unsigned char *key,
>> +			      char *value)
>> +{
>> +	bool single_line, break_names;
>> +	int value_size = strlen(value);
> 
> nit: order variables declaration lines to shortest, please.
> 
>> +
>> +	break_names = info->key_size > 16 || value_size > 16;
>> +	single_line = info->key_size + value_size <= 24 && !break_names;
>> +
>> +	printf("key:%c", break_names ? '\n' : ' ');
>> +	fprint_hex(stdout, key, info->key_size, " ");
>> +
>> +	printf(single_line ? "  " : "\n");
>> +
>> +	printf("value:%c%s", break_names ? '\n' : ' ', value);
>> +
>> +	printf("\n");
>> +}
>> +
>>   static void print_entry_plain(struct bpf_map_info *info, unsigned char *key,
>>   			      unsigned char *value)
>>   {
>> @@ -660,6 +679,8 @@ static int dump_map_elem(int fd, void *key, void *value,
>>   			 json_writer_t *btf_wtr)
>>   {
>>   	int num_elems = 0;
>> +	int lookup_errno;
>> +	char *errstr;
> 
> nit: const char?
> 
>>   
>>   	if (!bpf_map_lookup_elem(fd, key, value)) {
>>   		if (json_output) {
> 
> 
> 

^ permalink raw reply

* Re: [PATCH] netfilter: nf_tables: add SECMARK support
From: Florian Westphal @ 2018-09-20  8:50 UTC (permalink / raw)
  To: Casey Schaufler
  Cc: Christian Göttsche, pablo, kadlec, fw, davem,
	netfilter-devel, coreteam, netdev, linux-kernel, paul, sds,
	eparis, jmorris, serge, selinux, linux-security-module
In-Reply-To: <75b3fed9-e549-4ed0-c435-ec4795fc1e39@schaufler-ca.com>

Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 9/19/2018 4:14 PM, Christian Göttsche wrote:
> > Add the ability to set the security context of packets within the nf_tables framework.
> > Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.
> > The contexts are kept as strings and are evaluated to security identifiers at runtime (packet arrival),
> > so that the nft_objects do not need to be refreshed after security changes.
> > The maximum security context length is set to 256.
> >
> > Based on v4.18.6
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> 
> I've only had a cursory look at your patch, but how is it
> different from what's in xt_SECMARK.c ?

this change is supposed to make secmark labeling accessible from
nftables.

The advantage is that its now possible to use
maps to assign secmarks from a single rule instead of using
several rules:

nft add rule meta secmark set tcp dport map { 22 : tag-ssh, 80 :
	tag-http }

and so on.

> > +	for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
> > +		err = nft_register_obj(nft_basic_objects[i]);
> > +		if (err)
> > +			goto err;
> > +	}
> >  
> > -	for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
> > -		err = nft_register_expr(nft_basic_types[i]);
> > +	for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
> > +		err = nft_register_expr(nft_basic_types[j]);
> >  		if (err)
> >  			goto err;
> >  	}
> > @@ -248,8 +260,12 @@ int __init nf_tables_core_module_init(void)
> >  	return 0;
> >  
> >  err:
> > +	while (j-- > 0)
> > +		nft_unregister_expr(nft_basic_types[j]);
> > +
> >  	while (i-- > 0)
> > -		nft_unregister_expr(nft_basic_types[i]);
> > +		nft_unregister_obj(nft_basic_objects[i]);
> > +
> >  	return err;

Do I read this right in that this is a error unroll bug fix?
If so, could you please submit this as indepentent patch?

Fixes should go into nf.git whereas feature goes to nf-next.git.

> > +struct nft_secmark {
> > +	char ctx[NFT_SECMARK_CTX_MAXLEN];
> > +	int len;
> > +};
> > +
> > +static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
> > +	[NFTA_SECMARK_CTX]     = { .type = NLA_STRING, .len = NFT_SECMARK_CTX_MAXLEN },
> > +};
> > +
> > +static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs, const struct nft_pktinfo *pkt)
> > +{
> > +	const struct nft_secmark *priv = nft_obj_data(obj);
> > +	struct sk_buff *skb = pkt->skb;
> > +	int err;
> > +	u32 secid = 0;
> > +
> > +	/* skip if packet has already a secmark */
> > +	if (skb->secmark)
> > +		return;

xt_SECMARK doesn't do this and will allow relabeling.
What do the LSM experts think?

> > +	err = security_secctx_to_secid(priv->ctx, priv->len, &secid);

Could someone familiar with how LSMs work clarify if this has to be
called per-packet?

xt_SECMARK.c does this ctx -> secid mapping once, when the iptables rule
gets added, whereas this patch does it once for each packet.

Is the ctx -> secid mapping stable?

If yes, the code above should be moved to the ->init() hook, otherwise
we'll need to fix xt_SECMARK.c.

> > +	if (err) {
> > +		if (err == -EINVAL)
> > +			pr_notice_ratelimited("invalid security context \'%s\'\n", priv->ctx);
> > +		else
> > +			pr_notice_ratelimited("unable to convert security context \'%s\': %d\n", priv->ctx, -err);
> > +		return;
> > +	}

Please remove these printks(), they do not really help as user can't
take any action anyway.

> > +	err = security_secmark_relabel_packet(secid);

Hmm, this function uses current() to check permissions of calling
task, so this function canot be used in ->eval() path.

Network stack causes random results of "current()", as network
processing can "steal" cpu from some arbitrary task when
softinterrupt kicks in.

->init() is fine, as its in process context and current will be the task
installing the nftables rule.

^ permalink raw reply

* Re: [PATCH net 2/5] net/smc: remove duplicate mutex_unlock
From: Dan Carpenter @ 2018-09-20  9:12 UTC (permalink / raw)
  To: kbuild, Ursula Braun
  Cc: linux-s390, netdev, heiko.carstens, linux-kernel, kbuild-all,
	raspl, schwidefsky, davem
In-Reply-To: <20180918134638.90271-3-ubraun@linux.ibm.com>

Hi Ursula,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net/master]

url:    https://github.com/0day-ci/linux/commits/Ursula-Braun/net-smc-fixes-2018-09-18/20180919-080857

smatch warnings:
net/smc/af_smc.c:1289 smc_listen_work() warn: inconsistent returns 'mutex:&smc_create_lgr_pending'.
  Locked on:   line 1285
  Unlocked on: line 1209

# https://github.com/0day-ci/linux/commit/c69342ef9becfe90f3778db1c386abdd80b786d1
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout c69342ef9becfe90f3778db1c386abdd80b786d1
vim +1289 net/smc/af_smc.c

3b2dec260 Hans Wippel   2018-05-18  1231  	/* IPSec connections opt out of SMC-R optimizations */
3b2dec260 Hans Wippel   2018-05-18  1232  	if (using_ipsec(new_smc)) {
3b2dec260 Hans Wippel   2018-05-18  1233  		smc_listen_decline(new_smc, SMC_CLC_DECL_IPSEC, 0);
3b2dec260 Hans Wippel   2018-05-18  1234  		return;
3b2dec260 Hans Wippel   2018-05-18  1235  	}
3b2dec260 Hans Wippel   2018-05-18  1236  
3b2dec260 Hans Wippel   2018-05-18  1237  	mutex_lock(&smc_create_lgr_pending);
3b2dec260 Hans Wippel   2018-05-18  1238  	smc_close_init(new_smc);
3b2dec260 Hans Wippel   2018-05-18  1239  	smc_rx_init(new_smc);
3b2dec260 Hans Wippel   2018-05-18  1240  	smc_tx_init(new_smc);
3b2dec260 Hans Wippel   2018-05-18  1241  
413498440 Hans Wippel   2018-06-28  1242  	/* check if ISM is available */
413498440 Hans Wippel   2018-06-28  1243  	if ((pclc->hdr.path == SMC_TYPE_D || pclc->hdr.path == SMC_TYPE_B) &&
413498440 Hans Wippel   2018-06-28  1244  	    !smc_check_ism(new_smc, &ismdev) &&
413498440 Hans Wippel   2018-06-28  1245  	    !smc_listen_ism_init(new_smc, pclc, ismdev, &local_contact)) {
413498440 Hans Wippel   2018-06-28  1246  		ism_supported = true;
413498440 Hans Wippel   2018-06-28  1247  	}
413498440 Hans Wippel   2018-06-28  1248  
3b2dec260 Hans Wippel   2018-05-18  1249  	/* check if RDMA is available */
413498440 Hans Wippel   2018-06-28  1250  	if (!ism_supported &&
413498440 Hans Wippel   2018-06-28  1251  	    ((pclc->hdr.path != SMC_TYPE_R && pclc->hdr.path != SMC_TYPE_B) ||
7005ada68 Ursula Braun  2018-07-25  1252  	     smc_vlan_by_tcpsk(new_smc->clcsock, &vlan) ||
7005ada68 Ursula Braun  2018-07-25  1253  	     smc_check_rdma(new_smc, &ibdev, &ibport, vlan, NULL) ||
3b2dec260 Hans Wippel   2018-05-18  1254  	     smc_listen_rdma_check(new_smc, pclc) ||
3b2dec260 Hans Wippel   2018-05-18  1255  	     smc_listen_rdma_init(new_smc, pclc, ibdev, ibport,
3b2dec260 Hans Wippel   2018-05-18  1256  				  &local_contact) ||
413498440 Hans Wippel   2018-06-28  1257  	     smc_listen_rdma_reg(new_smc, local_contact))) {
3b2dec260 Hans Wippel   2018-05-18  1258  		/* SMC not supported, decline */
145686baa Ursula Braun  2017-10-25  1259  		mutex_unlock(&smc_create_lgr_pending);
603cc1498 Karsten Graul 2018-07-25  1260  		smc_listen_decline(new_smc, SMC_CLC_DECL_MODEUNSUPP,
603cc1498 Karsten Graul 2018-07-25  1261  				   local_contact);
3b2dec260 Hans Wippel   2018-05-18  1262  		return;
a046d57da Ursula Braun  2017-01-09  1263  	}
a046d57da Ursula Braun  2017-01-09  1264  
3b2dec260 Hans Wippel   2018-05-18  1265  	/* send SMC Accept CLC message */
3b2dec260 Hans Wippel   2018-05-18  1266  	rc = smc_clc_send_accept(new_smc, local_contact);
3b2dec260 Hans Wippel   2018-05-18  1267  	if (rc) {
145686baa Ursula Braun  2017-10-25  1268  		mutex_unlock(&smc_create_lgr_pending);
3b2dec260 Hans Wippel   2018-05-18  1269  		smc_listen_decline(new_smc, rc, local_contact);
3b2dec260 Hans Wippel   2018-05-18  1270  		return;
3b2dec260 Hans Wippel   2018-05-18  1271  	}
3b2dec260 Hans Wippel   2018-05-18  1272  
3b2dec260 Hans Wippel   2018-05-18  1273  	/* receive SMC Confirm CLC message */
3b2dec260 Hans Wippel   2018-05-18  1274  	reason_code = smc_clc_wait_msg(new_smc, &cclc, sizeof(cclc),
3b2dec260 Hans Wippel   2018-05-18  1275  				       SMC_CLC_CONFIRM);
3b2dec260 Hans Wippel   2018-05-18  1276  	if (reason_code) {
3b2dec260 Hans Wippel   2018-05-18  1277  		mutex_unlock(&smc_create_lgr_pending);
3b2dec260 Hans Wippel   2018-05-18  1278  		smc_listen_decline(new_smc, reason_code, local_contact);
3b2dec260 Hans Wippel   2018-05-18  1279  		return;
3b2dec260 Hans Wippel   2018-05-18  1280  	}
3b2dec260 Hans Wippel   2018-05-18  1281  
3b2dec260 Hans Wippel   2018-05-18  1282  	/* finish worker */
c69342ef9 Ursula Braun  2018-09-18  1283  	if (!ism_supported) {
c69342ef9 Ursula Braun  2018-09-18  1284  		if (smc_listen_rdma_finish(new_smc, &cclc, local_contact))
c69342ef9 Ursula Braun  2018-09-18  1285  			return;
                                                                ^^^^^^
We need to mutex_unlock(&smc_create_lgr_pending); before the return.

c69342ef9 Ursula Braun  2018-09-18  1286  	}
3b2dec260 Hans Wippel   2018-05-18  1287  	smc_conn_save_peer_info(new_smc, &cclc);
3b2dec260 Hans Wippel   2018-05-18  1288  	mutex_unlock(&smc_create_lgr_pending);
3b2dec260 Hans Wippel   2018-05-18 @1289  	smc_listen_out_connected(new_smc);
a046d57da Ursula Braun  2017-01-09  1290  }
a046d57da Ursula Braun  2017-01-09  1291  

^ permalink raw reply

* Re: [PATCH 00/11] pull request for net: batman-adv 2018-09-19
From: David Miller @ 2018-09-20  3:34 UTC (permalink / raw)
  To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20180919122213.23108-1-sw@simonwunderlich.de>

From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Wed, 19 Sep 2018 14:22:02 +0200

> here are some bugfixes which we would like to see integrated into net.
> 
> We forgot to bump the version number in the last round for net-next, so
> the belated patch to do that is included - we hope you can adopt it.
> This will most likely create a merge conflict later when merging into
> net-next with this rounds net-next patchset, but net-next should keep
> the 2018.4 version[1].
> 
> Please pull or let me know of any problem!

Pulled, thanks Simon.

In the future, you can put that merge resolution guidance into the
merge commit message.

When people give annotations like this, I refer to them when I do
merges and it helps me a lot.  If it's in the actual GIT history,
that is tons easier than if it just appeared in your pull request
email.

I manually included it in the merge commit message this time.

Thanks!

^ permalink raw reply

* Re: [PATCH 0/5] pull request for net-next: batman-adv 2018-09-19
From: David Miller @ 2018-09-20  3:36 UTC (permalink / raw)
  To: sw; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20180919123238.23742-1-sw@simonwunderlich.de>

From: Simon Wunderlich <sw@simonwunderlich.de>
Date: Wed, 19 Sep 2018 14:32:33 +0200

> here is a little feature and cleanup pull request of batman-adv to go into net-next.
> 
> Please pull or let me know of any problem!

Also pulled, thank you.

^ permalink raw reply

* Re: [PATCH] netfilter: nf_tables: add SECMARK support
From: Pablo Neira Ayuso @ 2018-09-20  9:30 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Casey Schaufler, Christian Göttsche, kadlec, davem,
	netfilter-devel, coreteam, netdev, linux-kernel, paul, sds,
	eparis, jmorris, serge, selinux, linux-security-module
In-Reply-To: <20180920085048.tps2v4jkko7zjav4@breakpoint.cc>

On Thu, Sep 20, 2018 at 10:50:48AM +0200, Florian Westphal wrote:
> Casey Schaufler <casey@schaufler-ca.com> wrote:
> > On 9/19/2018 4:14 PM, Christian Göttsche wrote:
> > > Add the ability to set the security context of packets within the nf_tables framework.
> > > Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.
> > > The contexts are kept as strings and are evaluated to security identifiers at runtime (packet arrival),
> > > so that the nft_objects do not need to be refreshed after security changes.
> > > The maximum security context length is set to 256.
> > >
> > > Based on v4.18.6
> > >
> > > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > 
> > I've only had a cursory look at your patch, but how is it
> > different from what's in xt_SECMARK.c ?
> 
> this change is supposed to make secmark labeling accessible from
> nftables.
> 
> The advantage is that its now possible to use
> maps to assign secmarks from a single rule instead of using
> several rules:
> 
> nft add rule meta secmark set tcp dport map { 22 : tag-ssh, 80 :
> 	tag-http }
> 
> and so on.
> 
> > > +	for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
> > > +		err = nft_register_obj(nft_basic_objects[i]);
> > > +		if (err)
> > > +			goto err;
> > > +	}
> > >  
> > > -	for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
> > > -		err = nft_register_expr(nft_basic_types[i]);
> > > +	for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
> > > +		err = nft_register_expr(nft_basic_types[j]);
> > >  		if (err)
> > >  			goto err;
> > >  	}
> > > @@ -248,8 +260,12 @@ int __init nf_tables_core_module_init(void)
> > >  	return 0;
> > >  
> > >  err:
> > > +	while (j-- > 0)
> > > +		nft_unregister_expr(nft_basic_types[j]);
> > > +
> > >  	while (i-- > 0)
> > > -		nft_unregister_expr(nft_basic_types[i]);
> > > +		nft_unregister_obj(nft_basic_objects[i]);
> > > +
> > >  	return err;
> 
> Do I read this right in that this is a error unroll bug fix?
> If so, could you please submit this as indepentent patch?
> 
> Fixes should go into nf.git whereas feature goes to nf-next.git.

nft_register_expr() never actually fails, so probably we can just turn
this into void.

@Christian: Please make sure you rebase your secmark patch on top of
nf-next.git.

^ permalink raw reply

* Re: [PATCH] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-20  9:32 UTC (permalink / raw)
  To: fw
  Cc: casey, pablo, kadlec, davem, netfilter-devel, coreteam, netdev,
	linux-kernel, Paul Moore, Stephen Smalley, Eric Paris, jmorris,
	serge, selinux, linux-security-module
In-Reply-To: <20180920085048.tps2v4jkko7zjav4@breakpoint.cc>

> > > +   for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
> > > +           err = nft_register_obj(nft_basic_objects[i]);
> > > +           if (err)
> > > +                   goto err;
> > > +   }
> > >
> > > -   for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
> > > -           err = nft_register_expr(nft_basic_types[i]);
> > > +   for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
> > > +           err = nft_register_expr(nft_basic_types[j]);
> > >             if (err)
> > >                     goto err;
> > >     }
> > > @@ -248,8 +260,12 @@ int __init nf_tables_core_module_init(void)
> > >     return 0;
> > >
> > >  err:
> > > +   while (j-- > 0)
> > > +           nft_unregister_expr(nft_basic_types[j]);
> > > +
> > >     while (i-- > 0)
> > > -           nft_unregister_expr(nft_basic_types[i]);
> > > +           nft_unregister_obj(nft_basic_objects[i]);
> > > +
> > >     return err;
>
> Do I read this right in that this is a error unroll bug fix?
> If so, could you please submit this as indepentent patch?
>
> Fixes should go into nf.git whereas feature goes to nf-next.git.

No, that should not be a unroll fix.
Currently there are no objects registered by the main nf_tables
module, so for nft_secmark_obj_type I had to introduce this new logic.

> > > +   if (err) {
> > > +           if (err == -EINVAL)
> > > +                   pr_notice_ratelimited("invalid security context \'%s\'\n", priv->ctx);
> > > +           else
> > > +                   pr_notice_ratelimited("unable to convert security context \'%s\': %d\n", priv->ctx, -err);
> > > +           return;
> > > +   }
>
> Please remove these printks(), they do not really help as user can't
> take any action anyway.

Aren't they helpful?
"invalid security context" can pop up if someone supplies an invalid
SELinux context (nft add secmark inet filter sshtag
\"this_is_invalid\") and uses it
"unable to convert security context" can pop up if no LSM is enabled
"unable to map security context" should never happen, but one never knows
"unable to obtain relabeling permission" can pop up if e.g. the
SELinux permission "kernel_t ssh_server_packet:packet relabelto" is
missing

^ permalink raw reply

* [PATCH 0/6] net: remove redundant null pointer check before kfree_skb
From: zhong jiang @ 2018-09-20  9:37 UTC (permalink / raw)
  To: davem; +Cc: kuznet, yoshfuji, santosh, netdev, linux-kernel

The issue is detected with the help of Coccinelle.

zhong jiang (6):
  net: tap: remove redundant null pointer check before kfree_skb
  net: cxgb3: remove redundant null pointer check before kfree_skb
  ipv4: remove redundant null pointer check before kfree_skb
  net: nci: remove redundant null pointer check before kfree_skb
  net: cxgb3_main: remove redundant null pointer check before kfree_skb
  ipv6: remove redundant null pointer check before kfree_skb

 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c    | 3 +--
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 3 +--
 drivers/net/tap.c                                  | 6 ++----
 net/ipv4/ip_fragment.c                             | 3 +--
 net/ipv6/af_inet6.c                                | 6 ++----
 net/nfc/nci/uart.c                                 | 6 ++----
 6 files changed, 9 insertions(+), 18 deletions(-)

-- 
1.7.12.4

^ permalink raw reply

* [PATCH 1/6] net: tap: remove redundant null pointer check before kfree_skb
From: zhong jiang @ 2018-09-20  9:37 UTC (permalink / raw)
  To: davem; +Cc: kuznet, yoshfuji, santosh, netdev, linux-kernel
In-Reply-To: <1537436266-41955-1-git-send-email-zhongjiang@huawei.com>

kfree_skb has taken the null pointer into account. hence it is safe
to remove the redundant null pointer check before kfree_skb.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/tap.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index a4ab4a7..f03004f 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -830,8 +830,7 @@ static ssize_t tap_do_read(struct tap_queue *q,
 	ssize_t ret = 0;
 
 	if (!iov_iter_count(to)) {
-		if (skb)
-			kfree_skb(skb);
+		kfree_skb(skb);
 		return 0;
 	}
 
@@ -1236,8 +1235,7 @@ static int tap_recvmsg(struct socket *sock, struct msghdr *m,
 	struct sk_buff *skb = m->msg_control;
 	int ret;
 	if (flags & ~(MSG_DONTWAIT|MSG_TRUNC)) {
-		if (skb)
-			kfree_skb(skb);
+		kfree_skb(skb);
 		return -EINVAL;
 	}
 	ret = tap_do_read(q, &m->msg_iter, flags & MSG_DONTWAIT, skb);
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 2/6] net: cxgb3: remove redundant null pointer check before kfree_skb
From: zhong jiang @ 2018-09-20  9:37 UTC (permalink / raw)
  To: davem; +Cc: kuznet, yoshfuji, santosh, netdev, linux-kernel
In-Reply-To: <1537436266-41955-1-git-send-email-zhongjiang@huawei.com>

kfree_skb has taken the null pointer into account. hence it is safe
to remove the redundant null pointer check before kfree_skb.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
index 50cd660..84604af 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_offload.c
@@ -1302,8 +1302,7 @@ void cxgb3_offload_deactivate(struct adapter *adapter)
 	rcu_read_unlock();
 	RCU_INIT_POINTER(tdev->l2opt, NULL);
 	call_rcu(&d->rcu_head, clean_l2_data);
-	if (t->nofail_skb)
-		kfree_skb(t->nofail_skb);
+	kfree_skb(t->nofail_skb);
 	kfree(t);
 }
 
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 3/6] ipv4: remove redundant null pointer check before kfree_skb
From: zhong jiang @ 2018-09-20  9:37 UTC (permalink / raw)
  To: davem; +Cc: kuznet, yoshfuji, santosh, netdev, linux-kernel
In-Reply-To: <1537436266-41955-1-git-send-email-zhongjiang@huawei.com>

kfree_skb has taken the null pointer into account. hence it is safe
to remove the redundant null pointer check before kfree_skb.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 net/ipv4/ip_fragment.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index da930b0..13f4d18 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -260,8 +260,7 @@ static void ip_expire(struct timer_list *t)
 	spin_unlock(&qp->q.lock);
 out_rcu_unlock:
 	rcu_read_unlock();
-	if (head)
-		kfree_skb(head);
+	kfree_skb(head);
 	ipq_put(qp);
 }
 
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 4/6] net: nci: remove redundant null pointer check before kfree_skb
From: zhong jiang @ 2018-09-20  9:37 UTC (permalink / raw)
  To: davem; +Cc: kuznet, yoshfuji, santosh, netdev, linux-kernel
In-Reply-To: <1537436266-41955-1-git-send-email-zhongjiang@huawei.com>

kfree_skb has taken the null pointer into account. hence it is safe
to remove the redundant null pointer check before kfree_skb.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 net/nfc/nci/uart.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/net/nfc/nci/uart.c b/net/nfc/nci/uart.c
index a66f102..4503937 100644
--- a/net/nfc/nci/uart.c
+++ b/net/nfc/nci/uart.c
@@ -192,10 +192,8 @@ static void nci_uart_tty_close(struct tty_struct *tty)
 	if (!nu)
 		return;
 
-	if (nu->tx_skb)
-		kfree_skb(nu->tx_skb);
-	if (nu->rx_skb)
-		kfree_skb(nu->rx_skb);
+	kfree_skb(nu->tx_skb);
+	kfree_skb(nu->rx_skb);
 
 	skb_queue_purge(&nu->tx_q);
 
-- 
1.7.12.4

^ permalink raw reply related

* [PATCH 5/6] net: cxgb3_main: remove redundant null pointer check before kfree_skb
From: zhong jiang @ 2018-09-20  9:37 UTC (permalink / raw)
  To: davem; +Cc: kuznet, yoshfuji, santosh, netdev, linux-kernel
In-Reply-To: <1537436266-41955-1-git-send-email-zhongjiang@huawei.com>

kfree_skb has taken the null pointer into account. hence it is safe
to remove the redundant null pointer check before kfree_skb.

Signed-off-by: zhong jiang <zhongjiang@huawei.com>
---
 drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index a19172d..680c6fe 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -3423,8 +3423,7 @@ static void remove_one(struct pci_dev *pdev)
 				free_netdev(adapter->port[i]);
 
 		iounmap(adapter->regs);
-		if (adapter->nofail_skb)
-			kfree_skb(adapter->nofail_skb);
+		kfree_skb(adapter->nofail_skb);
 		kfree(adapter);
 		pci_release_regions(pdev);
 		pci_disable_device(pdev);
-- 
1.7.12.4

^ permalink raw reply related


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