Netdev List
 help / color / mirror / Atom feed
* Re: WARNING: ODEBUG bug in netdev_freemem (2)
From: Thomas Gleixner @ 2019-06-24 17:27 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Eric Dumazet, syzbot, Alexander Duyck, amritha.nambiar,
	Andy Shevchenko, David Miller, Dmitry Torokhov, Florian Fainelli,
	Greg Kroah-Hartman, Ido Schimmel, LKML, netdev, syzkaller-bugs,
	tyhicks, wanghai26, yuehaibing
In-Reply-To: <alpine.DEB.2.21.1906241433020.32342@nanos.tec.linutronix.de>

On Mon, 24 Jun 2019, Thomas Gleixner wrote:
> On Mon, 24 Jun 2019, Dmitry Vyukov wrote:
> > On Mon, Jun 24, 2019 at 2:08 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > >>> ------------[ cut here ]------------
> > > >>> ODEBUG: free active (active state 0) object type: timer_list hint:
> > > >>> delayed_work_timer_fn+0x0/0x90 arch/x86/include/asm/paravirt.h:767
> > > >>
> > > >> One of the cleaned up devices has left an active timer which belongs to a
> > > >> delayed work. That's all I can decode out of that splat. :(
> > > >
> > > > Hi Thomas,
> > > >
> > > > If ODEBUG would memorize full stack traces for object allocation
> > > > (using lib/stackdepot.c), it would make this splat actionable, right?
> > > > I've fixed https://bugzilla.kernel.org/show_bug.cgi?id=203969 for this.
> > > >
> > >
> > > Not sure this would help in this case as some netdev are allocated through a generic helper.
> > >
> > > The driver specific portion might not show up in the stack trace.
> > >
> > > It would be nice here to get the work queue function pointer,
> > > so that it gives us a clue which driver needs a fix.
> 
> Hrm. Let me think about a way to achieve that after I handled that
> regression which is on my desk.

Here is a quick and dirty hack which solves the issue at least for all run
time initialized delayed work objects. Here is the output of a test I
whipped up for this:

 OBJ: Init delayed work, arm timer
 OBJ: Leak timer
 ODEBUG: free active (active state 0) object type: timer_list hint: delayed_work_timer_fn+0x0/0x20 chint: foo_fun+0x0/0x17

chint is the debug object hint of the compound object, i.e. the work
function 'foo_fun'.

Yes, naming sucks and there is still the option to use the existing
debug_obj::astate mechanics, but I was not able to wrap my head around all
the nasty corner cases which the workqueue code provides quickly. Needs
more thought.

Anyway, this should definitely help to diagnose the issue at hand.

Thanks,

	tglx

8<--------------------
 include/linux/debugobjects.h |   10 ++++++++++
 include/linux/workqueue.h    |   26 ++++++++++++++++----------
 kernel/workqueue.c           |    9 ++++++++-
 lib/debugobjects.c           |   43 +++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 75 insertions(+), 13 deletions(-)

--- a/include/linux/debugobjects.h
+++ b/include/linux/debugobjects.h
@@ -24,6 +24,9 @@ struct debug_obj_descr;
  * @astate:	current active state
  * @object:	pointer to the real object
  * @descr:	pointer to an object type specific debug description structure
+ * @comp_addr:	pointer to a compound object which is glued with @object
+ * @comp_descr:	pointer to a compound object type specific debug description
+ *		structure
  */
 struct debug_obj {
 	struct hlist_node	node;
@@ -31,6 +34,8 @@ struct debug_obj {
 	unsigned int		astate;
 	void			*object;
 	struct debug_obj_descr	*descr;
+	void			*comp_addr;
+	struct debug_obj_descr	*comp_descr;
 };
 
 /**
@@ -82,6 +87,9 @@ extern void
 debug_object_active_state(void *addr, struct debug_obj_descr *descr,
 			  unsigned int expect, unsigned int next);
 
+extern void debug_object_set_compound(void *addr, void *comp_addr,
+				      struct debug_obj_descr *comp_descr);
+
 extern void debug_objects_early_init(void);
 extern void debug_objects_mem_init(void);
 #else
@@ -99,6 +107,8 @@ static inline void
 debug_object_free      (void *addr, struct debug_obj_descr *descr) { }
 static inline void
 debug_object_assert_init(void *addr, struct debug_obj_descr *descr) { }
+static inline void
+debug_object_set_compound(void *addr, void *ca, struct debug_obj_descr *cd) { }
 
 static inline void debug_objects_early_init(void) { }
 static inline void debug_objects_mem_init(void) { }
--- a/include/linux/workqueue.h
+++ b/include/linux/workqueue.h
@@ -204,7 +204,7 @@ struct execute_work {
 	struct delayed_work n = __DELAYED_WORK_INITIALIZER(n, f, TIMER_DEFERRABLE)
 
 #ifdef CONFIG_DEBUG_OBJECTS_WORK
-extern void __init_work(struct work_struct *work, int onstack);
+extern void __init_work(struct work_struct *work, int onstack, bool delayed);
 extern void destroy_work_on_stack(struct work_struct *work);
 extern void destroy_delayed_work_on_stack(struct delayed_work *work);
 static inline unsigned int work_static(struct work_struct *work)
@@ -212,7 +212,7 @@ static inline unsigned int work_static(s
 	return *work_data_bits(work) & WORK_STRUCT_STATIC;
 }
 #else
-static inline void __init_work(struct work_struct *work, int onstack) { }
+static inline void __init_work(struct work_struct *work, int onstack, bool delayed) { }
 static inline void destroy_work_on_stack(struct work_struct *work) { }
 static inline void destroy_delayed_work_on_stack(struct delayed_work *work) { }
 static inline unsigned int work_static(struct work_struct *work) { return 0; }
@@ -226,20 +226,20 @@ static inline unsigned int work_static(s
  * to generate better code.
  */
 #ifdef CONFIG_LOCKDEP
-#define __INIT_WORK(_work, _func, _onstack)				\
+#define __INIT_WORK(_work, _func, _onstack, _delayed)			\
 	do {								\
 		static struct lock_class_key __key;			\
 									\
-		__init_work((_work), _onstack);				\
+		__init_work((_work), _onstack, _delayed);		\
 		(_work)->data = (atomic_long_t) WORK_DATA_INIT();	\
 		lockdep_init_map(&(_work)->lockdep_map, "(work_completion)"#_work, &__key, 0); \
 		INIT_LIST_HEAD(&(_work)->entry);			\
 		(_work)->func = (_func);				\
 	} while (0)
 #else
-#define __INIT_WORK(_work, _func, _onstack)				\
+#define __INIT_WORK(_work, _func, _onstack, _delayed)			\
 	do {								\
-		__init_work((_work), _onstack);				\
+		__init_work((_work), _onstack, _delayed);		\
 		(_work)->data = (atomic_long_t) WORK_DATA_INIT();	\
 		INIT_LIST_HEAD(&(_work)->entry);			\
 		(_work)->func = (_func);				\
@@ -247,25 +247,31 @@ static inline unsigned int work_static(s
 #endif
 
 #define INIT_WORK(_work, _func)						\
-	__INIT_WORK((_work), (_func), 0)
+	__INIT_WORK((_work), (_func), 0, 0)
 
 #define INIT_WORK_ONSTACK(_work, _func)					\
-	__INIT_WORK((_work), (_func), 1)
+	__INIT_WORK((_work), (_func), 1, 0)
+
+#define __INIT_DWORK(_work, _func)					\
+	__INIT_WORK((_work), (_func), 0, 1)
+
+#define __INIT_DWORK_ONSTACK(_work, _func)				\
+	__INIT_WORK((_work), (_func), 1, 1)
 
 #define __INIT_DELAYED_WORK(_work, _func, _tflags)			\
 	do {								\
-		INIT_WORK(&(_work)->work, (_func));			\
 		__init_timer(&(_work)->timer,				\
 			     delayed_work_timer_fn,			\
 			     (_tflags) | TIMER_IRQSAFE);		\
+		__INIT_DWORK(&(_work)->work, (_func));			\
 	} while (0)
 
 #define __INIT_DELAYED_WORK_ONSTACK(_work, _func, _tflags)		\
 	do {								\
-		INIT_WORK_ONSTACK(&(_work)->work, (_func));		\
 		__init_timer_on_stack(&(_work)->timer,			\
 				      delayed_work_timer_fn,		\
 				      (_tflags) | TIMER_IRQSAFE);	\
+		__INIT_DWORK_ONSTACK(&(_work)->work, (_func));		\
 	} while (0)
 
 #define INIT_DELAYED_WORK(_work, _func)					\
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -499,12 +499,19 @@ static inline void debug_work_deactivate
 	debug_object_deactivate(work, &work_debug_descr);
 }
 
-void __init_work(struct work_struct *work, int onstack)
+void __init_work(struct work_struct *work, int onstack, bool delayed)
 {
 	if (onstack)
 		debug_object_init_on_stack(work, &work_debug_descr);
 	else
 		debug_object_init(work, &work_debug_descr);
+
+	if (delayed) {
+		struct delayed_work *dwork = to_delayed_work(work);
+
+		debug_object_set_compound(&dwork->timer, work,
+					  &work_debug_descr);
+	}
 }
 EXPORT_SYMBOL_GPL(__init_work);
 
--- a/lib/debugobjects.c
+++ b/lib/debugobjects.c
@@ -179,6 +179,8 @@ alloc_object(void *addr, struct debug_bu
 		obj->descr  = descr;
 		obj->state  = ODEBUG_STATE_NONE;
 		obj->astate = 0;
+		obj->comp_addr = NULL;
+		obj->comp_descr = NULL;
 		hlist_del(&obj->node);
 
 		hlist_add_head(&obj->node, &b->list);
@@ -321,11 +323,17 @@ static void debug_print_object(struct de
 	if (limit < 5 && descr != descr_test) {
 		void *hint = descr->debug_hint ?
 			descr->debug_hint(obj->object) : NULL;
+		void *chint = NULL;
+
+		/* Get a hint about a compound object */
+		if (obj->comp_descr && obj->comp_descr->debug_hint)
+			chint = obj->comp_descr->debug_hint(obj->comp_addr);
+
 		limit++;
 		WARN(1, KERN_ERR "ODEBUG: %s %s (active state %u) "
-				 "object type: %s hint: %pS\n",
+				 "object type: %s hint: %pS chint: %pS\n",
 			msg, obj_states[obj->state], obj->astate,
-			descr->name, hint);
+		     descr->name, hint, chint);
 	}
 	debug_objects_warnings++;
 }
@@ -448,6 +456,37 @@ void debug_object_init_on_stack(void *ad
 EXPORT_SYMBOL_GPL(debug_object_init_on_stack);
 
 /**
+ * debug_object_set_compound - Set a pointer to a compund object
+ * @addr:	address of the object
+ * @comp_addr:	pointer to the compound object related to @addr
+ * @comp_descr:	pointer to an object specific debug description structure for
+ *		@comp_addr
+ *
+ * Useful for delayed work and similar constructs where the
+ * debug_obj::astate tracking would be complex to achieve.
+ */
+void debug_object_set_compound(void *addr, void *comp_addr,
+			       struct debug_obj_descr *comp_descr)
+{
+	struct debug_bucket *db;
+	struct debug_obj *obj;
+	unsigned long flags;
+
+	if (!debug_objects_enabled)
+		return;
+
+	db = get_bucket((unsigned long) addr);
+
+	raw_spin_lock_irqsave(&db->lock, flags);
+	obj = lookup_object(addr, db);
+	if (obj) {
+		obj->comp_addr = comp_addr;
+		obj->comp_descr = comp_descr;
+	}
+	raw_spin_unlock_irqrestore(&db->lock, flags);
+}
+
+/**
  * debug_object_activate - debug checks when an object is activated
  * @addr:	address of the object
  * @descr:	pointer to an object specific debug description structure

^ permalink raw reply

* Re: [PATCH net-next] can: dev: call netif_carrier_off() in register_candev()
From: Willem de Bruijn @ 2019-06-24 17:26 UTC (permalink / raw)
  To: Rasmus Villemoes
  Cc: Wolfgang Grandegger, Marc Kleine-Budde, David S. Miller,
	Rasmus Villemoes, linux-can@vger.kernel.org,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190624083352.29257-1-rasmus.villemoes@prevas.dk>

On Mon, Jun 24, 2019 at 4:34 AM Rasmus Villemoes
<rasmus.villemoes@prevas.dk> wrote:
>
> CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev
> trigger as suggested, there's a small inconsistency with the link
> property: The LED is on initially, stays on when the device is brought
> up, and then turns off (as expected) when the device is brought down.
>
> Make sure the LED always reflects the state of the CAN device.
>
> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@prevas.dk>

Should this target net? Regardless of CONFIG_CAN_LEDS deprecation,
this is already not initialized properly if that CONFIG is disabled
and a can_led_event call at device probe is a noop.

^ permalink raw reply

* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: David Miller @ 2019-06-24 17:22 UTC (permalink / raw)
  To: ndesaulniers; +Cc: nicolas.dichtel, netdev, lkp
In-Reply-To: <CAKwvOdmd2AooQrpPhBVhcRHGNsMoGFiXSyBA4_aBf7=oVeOx1g@mail.gmail.com>

From: Nick Desaulniers <ndesaulniers@google.com>
Date: Mon, 24 Jun 2019 10:17:03 -0700

> On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote:
>>
>> From: Nick Desaulniers <ndesaulniers@google.com>
>> Date: Mon, 24 Jun 2019 09:45:14 -0700
>>
>> > https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ
>>
>> Inaccessible...
>>
>>         This group either doesn't exist, or you don't have permission
>>         to access it. If you're sure this group exists, contact the
>>         Owner of the group and ask them to give you access.
> 
> Sorry, I set up the mailing list not too long ago, seem to have a long
> tail of permissions related issues.  I confirmed that the link was
> borked in an incognito window.  Via
> https://support.google.com/a/answer/9325317#Visibility I was able to
> change the obscure setting.  I now confirmed the above link works in
> an incognito window.  Thanks for reporting; can you please triple
> check?

Yep it works now, thanks.

>>
>> And you mean just changing to 'const' fixes something, how?
> 
> See the warning in the above link (assuming now you have access).
> Assigning a non-const variable the result of a function call that
> returns const discards the const qualifier.

Ok thanks for clarifying.

However I was speaking in terms of this fixing a functional bug rather
than a loss of const warning.

^ permalink raw reply

* Re: [PATCH iproute2 0/3] do not set IPv6-only options on IPv4 addresses
From: Stephen Hemminger @ 2019-06-24 17:20 UTC (permalink / raw)
  To: Andrea Claudi; +Cc: netdev, dsahern
In-Reply-To: <cover.1561394228.git.aclaudi@redhat.com>

On Mon, 24 Jun 2019 19:05:52 +0200
Andrea Claudi <aclaudi@redhat.com> wrote:

> 'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but
> it is possible to set them on IPv4 addresses, too. This should
> not be possible.
> 
> Fix this adding a check on the protocol family before setting
> the flags, and exiting with invarg() on error.
> 
> Andrea Claudi (3):
>   ip address: do not set nodad option for IPv4 addresses
>   ip address: do not set home option for IPv4 addresses
>   ip address: do not set mngtmpaddr option for IPv4 addresses
> 
>  ip/ipaddress.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 

Maybe this should be a warning, not a failure.
A little concerned that there will be some user with a scripted setup
that this breaks.

^ permalink raw reply

* Re: [PATCH net-next v7 00/11] Fix listing (IPv4, IPv6) and flushing (IPv6) of cached route exceptions
From: David Miller @ 2019-06-24 17:20 UTC (permalink / raw)
  To: sbrivio; +Cc: dsahern, jishi, weiwan, kafai, edumazet, matti.vaittinen, netdev
In-Reply-To: <cover.1561131177.git.sbrivio@redhat.com>

From: Stefano Brivio <sbrivio@redhat.com>
Date: Fri, 21 Jun 2019 17:45:19 +0200

> For IPv6 cached routes, the commands 'ip -6 route list cache' and
> 'ip -6 route flush cache' don't work at all after route exceptions have
> been moved to a separate hash table in commit 2b760fcf5cfb ("ipv6: hook
> up exception table to store dst cache").
> 
> For IPv4 cached routes, the command 'ip route list cache' has also
> stopped working in kernel 3.5 after commit 4895c771c7f0 ("ipv4: Add FIB
> nexthop exceptions.") introduced storage for route exceptions as a
> separate entity.
> 
> Fix this by allowing userspace to clearly request cached routes with
> the RTM_F_CLONED flag used as a filter (in conjuction with strict
> checking) and by retrieving and dumping cached routes if requested.
> 
> If strict checking is not requested (iproute2 < 5.0.0), we don't have a
> way to consistently filter results on other selectors (e.g. on tables),
> so skip filtering entirely and dump both regular routes and exceptions.
> 
> For IPv4, cache flushing uses a completely different mechanism, so it
> wasn't affected. Listing of exception routes (modified routes pre-3.5) was
> tested against these versions of kernel and iproute2:
 ...

Series applied, thanks.

^ permalink raw reply

* Re: [PATCH v2 net-next 4/4] cxgb4: Add MPS refcounting for alloc/free mac filters
From: kbuild test robot @ 2019-06-24 17:16 UTC (permalink / raw)
  To: Raju Rangoju; +Cc: kbuild-all, netdev, davem, nirranjan, dt, rajur
In-Reply-To: <20190624085037.2358-5-rajur@chelsio.com>

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

Hi Raju,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Raju-Rangoju/cxgb4-Reference-count-MPS-TCAM-entries-within-a-PF/20190624-230630
config: x86_64-rhel-7.2 (attached as .config)
compiler: clang version 9.0.0 (git://gitmirror/llvm_project fb2bd4a9398b35ee4f732ea0847d9c1226fc4cf3)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/net//ethernet/chelsio/cxgb4/cxgb4_mps.c:17:29: error: incompatible pointer types passing 'refcount_t *' (aka 'struct refcount_struct *') to parameter of type 'atomic_t *' [-Werror,-Wincompatible-pointer-types]
                           if (!atomic_dec_and_test(&mps_entry->refcnt)) {
                                                    ^~~~~~~~~~~~~~~~~~
   include/asm-generic/atomic-instrumented.h:745:31: note: passing argument to parameter 'v' here
   atomic_dec_and_test(atomic_t *v)
                                 ^
   1 error generated.

vim +17 drivers/net//ethernet/chelsio/cxgb4/cxgb4_mps.c

     5	
     6	static int cxgb4_mps_ref_dec_by_mac(struct adapter *adap,
     7					    const u8 *addr, const u8 *mask)
     8	{
     9		u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
    10		struct mps_entries_ref *mps_entry, *tmp;
    11		int ret = -EINVAL;
    12	
    13		spin_lock_bh(&adap->mps_ref_lock);
    14		list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
    15			if (ether_addr_equal(mps_entry->addr, addr) &&
    16			    ether_addr_equal(mps_entry->mask, mask ? mask : bitmask)) {
  > 17				if (!atomic_dec_and_test(&mps_entry->refcnt)) {
    18					spin_unlock_bh(&adap->mps_ref_lock);
    19					return -EBUSY;
    20				}
    21				list_del(&mps_entry->list);
    22				kfree(mps_entry);
    23				ret = 0;
    24				break;
    25			}
    26		}
    27		spin_unlock_bh(&adap->mps_ref_lock);
    28		return ret;
    29	}
    30	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 42604 bytes --]

^ permalink raw reply

* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: Nick Desaulniers @ 2019-06-24 17:17 UTC (permalink / raw)
  To: David Miller; +Cc: Nicolas Dichtel, netdev, kbuild test robot
In-Reply-To: <20190624.100609.1416082266723674267.davem@davemloft.net>

On Mon, Jun 24, 2019 at 10:06 AM David Miller <davem@davemloft.net> wrote:
>
> From: Nick Desaulniers <ndesaulniers@google.com>
> Date: Mon, 24 Jun 2019 09:45:14 -0700
>
> > https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ
>
> Inaccessible...
>
>         This group either doesn't exist, or you don't have permission
>         to access it. If you're sure this group exists, contact the
>         Owner of the group and ask them to give you access.

Sorry, I set up the mailing list not too long ago, seem to have a long
tail of permissions related issues.  I confirmed that the link was
borked in an incognito window.  Via
https://support.google.com/a/answer/9325317#Visibility I was able to
change the obscure setting.  I now confirmed the above link works in
an incognito window.  Thanks for reporting; can you please triple
check?

>
> And you mean just changing to 'const' fixes something, how?

See the warning in the above link (assuming now you have access).
Assigning a non-const variable the result of a function call that
returns const discards the const qualifier.

-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* Re: KASAN: global-out-of-bounds Read in qmi_wwan_probe
From: Kristian Evensen @ 2019-06-24 17:16 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Hillf Danton, syzbot, andreyknvl, David Miller, linux-kernel,
	linux-usb, Network Development, syzkaller-bugs
In-Reply-To: <87tvcf54qc.fsf@miraculix.mork.no>

Hi,

On Mon, Jun 24, 2019 at 6:26 PM Bjørn Mork <bjorn@mork.no> wrote:
> Doh! Right you are.  Thanks to both you and Andrey for quick and good
> help.
>
> We obviously have some bad code patterns here, since this apparently
> worked for Kristian by pure luck.

Thanks a lot to everyone for spotting and fixing my mistake, and sorry
for not replying earlier. The patch from Bjørn is probably a candidate
for stable as well. I don't remember exactly when the quirk was
accepted in the kernel, but I recently submitted and got the quirk
accepted into 4.14.

BR,
Kristian

^ permalink raw reply

* Re: [PATCH net,stable] qmi_wwan: Fix out-of-bounds read
From: David Miller @ 2019-06-24 17:09 UTC (permalink / raw)
  To: bjorn; +Cc: netdev, linux-usb, hdanton, kristian.evensen
In-Reply-To: <20190624164511.831-1-bjorn@mork.no>

From: Bjørn Mork <bjorn@mork.no>
Date: Mon, 24 Jun 2019 18:45:11 +0200

> The syzbot reported
> 
>  Call Trace:
>   __dump_stack lib/dump_stack.c:77 [inline]
>   dump_stack+0xca/0x13e lib/dump_stack.c:113
>   print_address_description+0x67/0x231 mm/kasan/report.c:188
>   __kasan_report.cold+0x1a/0x32 mm/kasan/report.c:317
>   kasan_report+0xe/0x20 mm/kasan/common.c:614
>   qmi_wwan_probe+0x342/0x360 drivers/net/usb/qmi_wwan.c:1417
>   usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
>   really_probe+0x281/0x660 drivers/base/dd.c:509
>   driver_probe_device+0x104/0x210 drivers/base/dd.c:670
>   __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:777
>   bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454
> 
> Caused by too many confusing indirections and casts.
> id->driver_info is a pointer stored in a long.  We want the
> pointer here, not the address of it.
> 
> Thanks-to: Hillf Danton <hdanton@sina.com>
> Reported-by: syzbot+b68605d7fadd21510de1@syzkaller.appspotmail.com
> Cc: Kristian Evensen <kristian.evensen@gmail.com>
> Fixes: e4bf63482c30 ("qmi_wwan: Add quirk for Quectel dynamic config")
> Signed-off-by: Bjørn Mork <bjorn@mork.no>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH v2 net-next 0/4] cxgb4: Reference count MPS TCAM entries within a PF
From: Raju Rangoju @ 2019-06-24 17:06 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, nirranjan, dt
In-Reply-To: <20190624.075323.2257534731180163594.davem@davemloft.net>

On Monday, June 06/24/19, 2019 at 07:53:23 -0700, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Mon, 24 Jun 2019 07:51:32 -0700 (PDT)
> 
> > From: Raju Rangoju <rajur@chelsio.com>
> > Date: Mon, 24 Jun 2019 14:20:33 +0530
> > 
> >> Firmware reference counts the MPS TCAM entries by PF and VF,
> >> but it does not do it for usage within a PF or VF. This patch
> >> adds the support to track MPS TCAM entries within a PF.
> >> 
> >> v1->v2:
> >>  Use refcount_t type instead of atomic_t for mps reference count
> > 
> > Series applied, thanks.
> 
> Umm, REALLY?!?!?!
> 
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c: In function ‘cxgb4_mps_ref_dec_by_mac’:
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:17:29: error: passing argument 1 of ‘atomic_dec_and_test’ from incompatible pointer type [-Werror=incompatible-pointer-types]
>     if (!atomic_dec_and_test(&mps_entry->refcnt)) {
>                              ^~~~~~~~~~~~~~~~~~
> 
> You just changed it to a refcount_t and didn't try compiling the
> result?
> 

No. I'm pretty sure that I have compiled and tested the changes. But, my bad
I had missed to '--amend' the last patch after 'git add'.

I'll send out next version.

> The whole point of refcount_t is that it uses a different set of
> interfaces to manipulate the object and you have to therefore
> update all the call sites properly.
> 
> Reverted...

^ permalink raw reply

* Re: WWAN Controller Framework (was IPA [PATCH v2 00/17])
From: Alex Elder @ 2019-06-24 17:06 UTC (permalink / raw)
  To: davem, arnd, bjorn.andersson, ilias.apalodimas, Dan Williams,
	Johannes Berg
  Cc: evgreen, benchan, ejcaruso, cpratapa, syadagir, subashab,
	abhishek.esse, netdev, devicetree, linux-kernel, linux-soc,
	linux-arm-kernel, linux-arm-msm
In-Reply-To: <23ff4cce-1fee-98ab-3608-1fd09c2d97f1@linaro.org>

Sorry, I neglected to add Dan and Johannes--who have been
primary contributors in this discussion--to this.  Adding now.

					-Alex

On 6/24/19 11:30 AM, Alex Elder wrote:
> OK I want to try to organize a little more concisely some of the
> discussion on this, because there is a very large amount of volume
> to date and I think we need to try to narrow the focus back down
> again.
> 
> I'm going to use a few terms here.  Some of these I really don't
> like, but I want to be unambiguous *and* (at least for now) I want
> to avoid the very overloaded term "device".
> 
> I have lots more to say, but let's start with a top-level picture,
> to make sure we're all on the same page.
> 
>          WWAN Communication
>          Channel (Physical)
>                  |     ------------------------
> ------------     v     |           :+ Control |  \
> |          |-----------|           :+ Data    |  |
> |    AP    |           | WWAN unit :+ Voice   |   > Functions
> |          |===========|           :+ GPS     |  |
> ------------     ^     |           :+ ...     |  /
>                  |     -------------------------
>           Multiplexed WWAN
>            Communication
>          Channel (Physical)
> 
> - The *AP* is the main CPU complex that's running Linux on one or
>   more CPU cores.
> - A *WWAN unit* is an entity that shares one or more physical
>   *WWAN communication channels* with the AP.
> - A *WWAN communication channel* is a bidirectional means of
>   carrying data between the AP and WWAN unit.
> - A WWAN communication channel carries data using a *WWAN protocol*.
> - A WWAN unit implements one or more *WWAN functions*, such as
>   5G data, LTE voice, GPS, and so on.
> - A WWAN unit shall implement a *WWAN control function*, used to
>   manage the use of other WWAN functions, as well as the WWAN unit
>   itself.
> - The AP communicates with a WWAN function using a WWAN protocol.
> - A WWAN physical channel can be *multiplexed*, in which case it
>   carries the data for one or more *WWAN logical channels*.
> - A multiplexed WWAN communication channel uses a *WWAN wultiplexing
>   protocol*, which is used to separate independent data streams
>   carrying other WWAN protocols.
> - A WWAN logical channel carries a bidirectional stream of WWAN
>   protocol data between an entity on the AP and a WWAN function.
> 
> Does that adequately represent a very high-level picture of what
> we're trying to manage?
> 
> And if I understand it right, the purpose of the generic framework
> being discussed is to define a common mechanism for managing (i.e.,
> discovering, creating, destroying, querying, configuring, enabling,
> disabling, etc.) WWAN units and the functions they implement, along
> with the communication and logical channels used to communicate with
> them.
> 
> Comments?
> 
> 					-Alex
> 


^ permalink raw reply

* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: David Miller @ 2019-06-24 17:06 UTC (permalink / raw)
  To: ndesaulniers; +Cc: nicolas.dichtel, netdev, lkp
In-Reply-To: <CAKwvOdk9yxnO_2yDwuG8ECw2o8kP=w8pvdbCqDuwO4_03rj5gw@mail.gmail.com>

From: Nick Desaulniers <ndesaulniers@google.com>
Date: Mon, 24 Jun 2019 09:45:14 -0700

> https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ

Inaccessible...

	This group either doesn't exist, or you don't have permission
	to access it. If you're sure this group exists, contact the
	Owner of the group and ask them to give you access.

And you mean just changing to 'const' fixes something, how?

^ permalink raw reply

* [PATCH iproute2 2/3] ip address: do not set home option for IPv4 addresses
From: Andrea Claudi @ 2019-06-24 17:05 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern
In-Reply-To: <cover.1561394228.git.aclaudi@redhat.com>

'home' option designates a IPv6 address as "home address" as
defined in RFC 6275. This option should be available only for
IPv6 addresses, as correctly stated in the manpage.

However it is possible to set home on IPv4 addresses, too:

$ ip link add dummy0 type dummy
$ ip -4 addr add 192.168.1.1 dev dummy0 home
$ ip a
1: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
   link/ether 1a:6d:c6:96:ca:f8 brd ff:ff:ff:ff:ff:ff
   inet 192.168.1.1/32 scope global home dummy0
      valid_lft forever preferred_lft forever

Fix this adding a check on the protocol family before setting
IFA_F_HOMEADDRESS flag.

Fixes: bac735c53a36d ("enabled to manipulate the flags of IFA_F_HOMEADDRESS or IFA_F_NODAD from ip.")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 ip/ipaddress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 38356cc929e7b..0f59e0a40468c 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2248,7 +2248,10 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			if (set_lifetime(&preferred_lft, *argv))
 				invarg("preferred_lft value", *argv);
 		} else if (strcmp(*argv, "home") == 0) {
-			ifa_flags |= IFA_F_HOMEADDRESS;
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_HOMEADDRESS;
+			else
+				invarg("home option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "nodad") == 0) {
 			if (req.ifa.ifa_family == AF_INET6)
 				ifa_flags |= IFA_F_NODAD;
-- 
2.20.1


^ permalink raw reply related

* [PATCH iproute2 3/3] ip address: do not set mngtmpaddr option for IPv4 addresses
From: Andrea Claudi @ 2019-06-24 17:05 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern
In-Reply-To: <cover.1561394228.git.aclaudi@redhat.com>

'mngtmpaddr' option make the kernel manage temporary addresses
created from the specified one as template on behalf of Privacy
Extensions (RFC3041). This option should be available only for
IPv6 addresses, as correctly stated in the manpage.

However it is possible to set mngtmpaddr on IPv4 addresses, too:

$ ip link add dummy0 type dummy
$ ip -4 addr add 192.168.1.1 dev dummy0 mngtmpaddr
$ ip a
1: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
   link/ether 1a:6d:c6:96:ca:f8 brd ff:ff:ff:ff:ff:ff
   inet 192.168.1.1/32 scope global mngtmpaddr dummy0
      valid_lft forever preferred_lft forever

Fix this adding a check on the protocol family before setting
IFA_F_MANAGETEMPADDR flag.

Fixes: 5b7e21c417bea ("add support for IFA_F_MANAGETEMPADDR")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 ip/ipaddress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 0f59e0a40468c..06a9f904201c0 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2258,7 +2258,10 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 			else
 				invarg("nodad option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "mngtmpaddr") == 0) {
-			ifa_flags |= IFA_F_MANAGETEMPADDR;
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_MANAGETEMPADDR;
+			else
+				invarg("mngtmpaddr option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "noprefixroute") == 0) {
 			ifa_flags |= IFA_F_NOPREFIXROUTE;
 		} else if (strcmp(*argv, "autojoin") == 0) {
-- 
2.20.1


^ permalink raw reply related

* [PATCH iproute2 1/3] ip address: do not set nodad option for IPv4 addresses
From: Andrea Claudi @ 2019-06-24 17:05 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern
In-Reply-To: <cover.1561394228.git.aclaudi@redhat.com>

Duplicate Address Detection (RFC 4862) is available only for IPv6
addresses. As a consequence, 'nodad' option, turning it off, should
be available only for IPv6, and is defined like that in the man page.

However it is possible to set nodad on IPv4 addresses, too:

$ ip link add dummy0 type dummy
$ ip -4 addr add 192.168.1.1 dev dummy0 nodad
$ ip a
1: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default qlen 1000
   link/ether 1a:6d:c6:96:ca:f8 brd ff:ff:ff:ff:ff:ff
   inet 192.168.1.1/32 scope global nodad dummy0
      valid_lft forever preferred_lft forever

Fix this adding a check on the protocol family before setting
IFA_F_NODAD flag.

Fixes: bac735c53a36d ("enabled to manipulate the flags of IFA_F_HOMEADDRESS or IFA_F_NODAD from ip.")
Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
---
 ip/ipaddress.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 47e5be7462fe7..38356cc929e7b 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -2250,7 +2250,10 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
 		} else if (strcmp(*argv, "home") == 0) {
 			ifa_flags |= IFA_F_HOMEADDRESS;
 		} else if (strcmp(*argv, "nodad") == 0) {
-			ifa_flags |= IFA_F_NODAD;
+			if (req.ifa.ifa_family == AF_INET6)
+				ifa_flags |= IFA_F_NODAD;
+			else
+				invarg("nodad option can be set only for IPv6 addresses\n", *argv);
 		} else if (strcmp(*argv, "mngtmpaddr") == 0) {
 			ifa_flags |= IFA_F_MANAGETEMPADDR;
 		} else if (strcmp(*argv, "noprefixroute") == 0) {
-- 
2.20.1


^ permalink raw reply related

* [PATCH iproute2 0/3] do not set IPv6-only options on IPv4 addresses
From: Andrea Claudi @ 2019-06-24 17:05 UTC (permalink / raw)
  To: netdev; +Cc: stephen, dsahern

'home', 'nodad' and 'mngtmpaddr' options are IPv6-only, but
it is possible to set them on IPv4 addresses, too. This should
not be possible.

Fix this adding a check on the protocol family before setting
the flags, and exiting with invarg() on error.

Andrea Claudi (3):
  ip address: do not set nodad option for IPv4 addresses
  ip address: do not set home option for IPv4 addresses
  ip address: do not set mngtmpaddr option for IPv4 addresses

 ip/ipaddress.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

-- 
2.20.1


^ permalink raw reply

* Re: [PATCHv2 net] tipc: check msg->req data len in tipc_nl_compat_bearer_disable
From: David Miller @ 2019-06-24 17:04 UTC (permalink / raw)
  To: lucien.xin
  Cc: netdev, edumazet, jon.maloy, ying.xue, tipc-discussion,
	syzkaller-bugs
In-Reply-To: <58c46f0c73a4c1aea970e52de69188e2dd20d3b4.1561393699.git.lucien.xin@gmail.com>

From: Xin Long <lucien.xin@gmail.com>
Date: Tue, 25 Jun 2019 00:28:19 +0800

> This patch is to fix an uninit-value issue, reported by syzbot:
 ...
> TLV_GET_DATA_LEN() may return a negtive int value, which will be
> used as size_t (becoming a big unsigned long) passed into memchr,
> cause this issue.
> 
> Similar to what it does in tipc_nl_compat_bearer_enable(), this
> fix is to return -EINVAL when TLV_GET_DATA_LEN() is negtive in
> tipc_nl_compat_bearer_disable(), as well as in
> tipc_nl_compat_link_stat_dump() and tipc_nl_compat_link_reset_stats().
> 
> v1->v2:
>   - add the missing Fixes tags per Eric's request.
> 
> Fixes: 0762216c0ad2 ("tipc: fix uninit-value in tipc_nl_compat_bearer_enable")
> Fixes: 8b66fee7f8ee ("tipc: fix uninit-value in tipc_nl_compat_link_reset_stats")
> Reported-by: syzbot+30eaa8bf392f7fafffaf@syzkaller.appspotmail.com
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH rdma-next v1 12/12] IB/mlx5: Add DEVX support for CQ events
From: Yishai Hadas @ 2019-06-24 17:03 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
	Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20190624120416.GE5479@mellanox.com>

On 6/24/2019 3:04 PM, Jason Gunthorpe wrote:
> On Tue, Jun 18, 2019 at 08:15:40PM +0300, Leon Romanovsky wrote:
>> From: Yishai Hadas <yishaih@mellanox.com>
>>
>> Add DEVX support for CQ events by creating and destroying the CQ via
>> mlx5_core and set an handler to manage its completions.
>>
>> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>>   drivers/infiniband/hw/mlx5/devx.c | 40 +++++++++++++++++++++++++++++++
>>   1 file changed, 40 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
>> index 49fdce95d6d9..91ccd58ebc05 100644
>> +++ b/drivers/infiniband/hw/mlx5/devx.c
>> @@ -19,9 +19,12 @@
>>   #define UVERBS_MODULE_NAME mlx5_ib
>>   #include <rdma/uverbs_named_ioctl.h>
>>   
>> +static void dispatch_event_fd(struct list_head *fd_list, const void *data);
>> +
>>   enum devx_obj_flags {
>>   	DEVX_OBJ_FLAGS_INDIRECT_MKEY = 1 << 0,
>>   	DEVX_OBJ_FLAGS_DCT = 1 << 1,
>> +	DEVX_OBJ_FLAGS_CQ = 1 << 2,
>>   };
>>   
>>   struct devx_async_data {
>> @@ -94,6 +97,7 @@ struct devx_async_event_file {
>>   #define MLX5_MAX_DESTROY_INBOX_SIZE_DW MLX5_ST_SZ_DW(delete_fte_in)
>>   struct devx_obj {
>>   	struct mlx5_core_dev	*mdev;
>> +	struct mlx5_ib_dev	*ib_dev;
> 
> This seems strange, why would we need to store the core_dev and the ib_dev
> in a struct when ibdev->mdev == core_dev?
> 

We need to add the ib_dev as we can't access it from the core_dev.
Most of this patch we can probably go and drop the mdev and access it 
from ib_dev, I preferred to not handle that in this patch.


^ permalink raw reply

* Re: [PATCH v2 net-next 4/4] cxgb4: Add MPS refcounting for alloc/free mac filters
From: kbuild test robot @ 2019-06-24 16:59 UTC (permalink / raw)
  To: Raju Rangoju; +Cc: kbuild-all, netdev, davem, nirranjan, dt, rajur
In-Reply-To: <20190624085037.2358-5-rajur@chelsio.com>

Hi Raju,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Raju-Rangoju/cxgb4-Reference-count-MPS-TCAM-entries-within-a-PF/20190624-230630
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.1-rc1-7-g2b96cd8-dirty
        make ARCH=x86_64 allmodconfig
        make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)

>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:17:51: sparse: sparse: incorrect type in argument 1 (different base types) @@    expected struct atomic_t [usertype] *v @@    got ct atomic_t [usertype] *v @@
>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:17:51: sparse:    expected struct atomic_t [usertype] *v
>> drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c:17:51: sparse:    got struct refcount_struct *

vim +17 drivers/net/ethernet/chelsio/cxgb4/cxgb4_mps.c

     5	
     6	static int cxgb4_mps_ref_dec_by_mac(struct adapter *adap,
     7					    const u8 *addr, const u8 *mask)
     8	{
     9		u8 bitmask[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff };
    10		struct mps_entries_ref *mps_entry, *tmp;
    11		int ret = -EINVAL;
    12	
    13		spin_lock_bh(&adap->mps_ref_lock);
    14		list_for_each_entry_safe(mps_entry, tmp, &adap->mps_ref, list) {
    15			if (ether_addr_equal(mps_entry->addr, addr) &&
    16			    ether_addr_equal(mps_entry->mask, mask ? mask : bitmask)) {
  > 17				if (!atomic_dec_and_test(&mps_entry->refcnt)) {
    18					spin_unlock_bh(&adap->mps_ref_lock);
    19					return -EBUSY;
    20				}
    21				list_del(&mps_entry->list);
    22				kfree(mps_entry);
    23				ret = 0;
    24				break;
    25			}
    26		}
    27		spin_unlock_bh(&adap->mps_ref_lock);
    28		return ret;
    29	}
    30	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply

* Re: [PATCH rdma-next v1 11/12] IB/mlx5: Implement DEVX dispatching event
From: Yishai Hadas @ 2019-06-24 16:55 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Leon Romanovsky, Doug Ledford, Leon Romanovsky, RDMA mailing list,
	Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20190624120338.GD5479@mellanox.com>

On 6/24/2019 3:03 PM, Jason Gunthorpe wrote:
> On Tue, Jun 18, 2019 at 08:15:39PM +0300, Leon Romanovsky wrote:
>> From: Yishai Hadas <yishaih@mellanox.com>
>>
>> Implement DEVX dispatching event by looking up for the applicable
>> subscriptions for the reported event and using their target fd to
>> signal/set the event.
>>
>> Signed-off-by: Yishai Hadas <yishaih@mellanox.com>
>> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>>   drivers/infiniband/hw/mlx5/devx.c         | 362 +++++++++++++++++++++-
>>   include/uapi/rdma/mlx5_user_ioctl_verbs.h |   5 +
>>   2 files changed, 357 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/mlx5/devx.c b/drivers/infiniband/hw/mlx5/devx.c
>> index 304b13e7a265..49fdce95d6d9 100644
>> +++ b/drivers/infiniband/hw/mlx5/devx.c
>> @@ -34,6 +34,11 @@ struct devx_async_data {
>>   	struct mlx5_ib_uapi_devx_async_cmd_hdr hdr;
>>   };
>>   
>> +struct devx_async_event_data {
>> +	struct list_head list; /* headed in ev_queue->event_list */
>> +	struct mlx5_ib_uapi_devx_async_event_hdr hdr;
>> +};
>> +
>>   /* first level XA value data structure */
>>   struct devx_event {
>>   	struct xarray object_ids; /* second XA level, Key = object id */
>> @@ -54,7 +59,9 @@ struct devx_event_subscription {
>>   				   * devx_obj_event->obj_sub_list
>>   				   */
>>   	struct list_head obj_list; /* headed in devx_object */
>> +	struct list_head event_list; /* headed in ev_queue->event_list */
>>   
>> +	u8  is_cleaned:1;
> 
> There is a loose bool 'is_obj_related' that should be combined with
> this bool bitfield as well.
> 

OK

>>   static void devx_cleanup_subscription(struct mlx5_ib_dev *dev,
>> -				      struct devx_event_subscription *sub)
>> +				      struct devx_event_subscription *sub,
>> +				      bool file_close)
>>   {
>> -	list_del_rcu(&sub->file_list);
>> +	if (sub->is_cleaned)
>> +		goto end;
>> +
>> +	sub->is_cleaned = 1;
>>   	list_del_rcu(&sub->xa_list);
>>   
>>   	if (sub->is_obj_related) {
>> @@ -1303,10 +1355,15 @@ static void devx_cleanup_subscription(struct mlx5_ib_dev *dev,
>>   		}
>>   	}
>>   
>> -	if (sub->eventfd)
>> -		eventfd_ctx_put(sub->eventfd);
>> +end:
>> +	if (file_close) {
>> +		if (sub->eventfd)
>> +			eventfd_ctx_put(sub->eventfd);
>>   
>> -	kfree_rcu(sub, rcu);
>> +		list_del_rcu(&sub->file_list);
>> +		/* subscription may not be used by the read API any more */
>> +		kfree_rcu(sub, rcu);
>> +	}
> 
> Dis like this confusing file_close stuff, just put this in the single place
> that calls this with the true bool
> 

OK, will do.

>> +static int deliver_event(struct devx_event_subscription *event_sub,
>> +			 const void *data)
>> +{
>> +	struct ib_uobject *fd_uobj = event_sub->fd_uobj;
>> +	struct devx_async_event_file *ev_file;
>> +	struct devx_async_event_queue *ev_queue;
>> +	struct devx_async_event_data *event_data;
>> +	unsigned long flags;
>> +	bool omit_data;
>> +
>> +	ev_file = container_of(fd_uobj, struct devx_async_event_file,
>> +			       uobj);
>> +	ev_queue = &ev_file->ev_queue;
>> +	omit_data = ev_queue->flags &
>> +		MLX5_IB_UAPI_DEVX_CREATE_EVENT_CHANNEL_FLAGS_OMIT_EV_DATA;
>> +
>> +	if (omit_data) {
>> +		spin_lock_irqsave(&ev_queue->lock, flags);
>> +		if (!list_empty(&event_sub->event_list)) {
>> +			spin_unlock_irqrestore(&ev_queue->lock, flags);
>> +			return 0;
>> +		}
>> +
>> +		list_add_tail(&event_sub->event_list, &ev_queue->event_list);
>> +		spin_unlock_irqrestore(&ev_queue->lock, flags);
>> +		wake_up_interruptible(&ev_queue->poll_wait);
>> +		return 0;
>> +	}
>> +
>> +	event_data = kzalloc(sizeof(*event_data) +
>> +			     (omit_data ? 0 : sizeof(struct mlx5_eqe)),
>> +			     GFP_ATOMIC);
> 
> omit_data is always false here
> 

Correct, will clean it up.

>> +	if (!event_data) {
>> +		spin_lock_irqsave(&ev_queue->lock, flags);
>> +		ev_queue->is_overflow_err = 1;
>> +		spin_unlock_irqrestore(&ev_queue->lock, flags);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	event_data->hdr.cookie = event_sub->cookie;
>> +	memcpy(event_data->hdr.out_data, data, sizeof(struct mlx5_eqe));
>> +
>> +	spin_lock_irqsave(&ev_queue->lock, flags);
>> +	list_add_tail(&event_data->list, &ev_queue->event_list);
>> +	spin_unlock_irqrestore(&ev_queue->lock, flags);
>> +	wake_up_interruptible(&ev_queue->poll_wait);
>> +
>> +	return 0;
>> +}
>> +
>> +static void dispatch_event_fd(struct list_head *fd_list,
>> +			      const void *data)
>> +{
>> +	struct devx_event_subscription *item;
>> +
>> +	list_for_each_entry_rcu(item, fd_list, xa_list) {
>> +		if (!get_file_rcu((struct file *)item->object))
>> +			continue;
>> +
>> +		if (item->eventfd) {
>> +			eventfd_signal(item->eventfd, 1);
>> +			fput(item->object);
>> +			continue;
>> +		}
>> +
>> +		deliver_event(item, data);
>> +		fput(item->object);
>> +	}
>> +}
>> +
>>   static int devx_event_notifier(struct notifier_block *nb,
>>   			       unsigned long event_type, void *data)
>>   {
>> -	return NOTIFY_DONE;
>> +	struct mlx5_devx_event_table *table;
>> +	struct mlx5_ib_dev *dev;
>> +	struct devx_event *event;
>> +	struct devx_obj_event *obj_event;
>> +	u16 obj_type = 0;
>> +	bool is_unaffiliated;
>> +	u32 obj_id;
>> +
>> +	/* Explicit filtering to kernel events which may occur frequently */
>> +	if (event_type == MLX5_EVENT_TYPE_CMD ||
>> +	    event_type == MLX5_EVENT_TYPE_PAGE_REQUEST)
>> +		return NOTIFY_OK;
>> +
>> +	table = container_of(nb, struct mlx5_devx_event_table, devx_nb.nb);
>> +	dev = container_of(table, struct mlx5_ib_dev, devx_event_table);
>> +	is_unaffiliated = is_unaffiliated_event(dev->mdev, event_type);
>> +
>> +	if (!is_unaffiliated)
>> +		obj_type = get_event_obj_type(event_type, data);
>> +	event = xa_load(&table->event_xa, event_type | (obj_type << 16));
>> +	if (!event)
>> +		return NOTIFY_DONE;
> 
> event should be in the rcu as well

Do we really need this ? I didn't see a flow that really requires that.
> 
>> +	if (is_unaffiliated) {
>> +		dispatch_event_fd(&event->unaffiliated_list, data);
>> +		return NOTIFY_OK;
>> +	}
>> +
>> +	obj_id = devx_get_obj_id_from_event(event_type, data);
>> +	rcu_read_lock();
>> +	obj_event = xa_load(&event->object_ids, obj_id);
>> +	if (!obj_event) {
>> +		rcu_read_unlock();
>> +		return NOTIFY_DONE;
>> +	}
>> +
>> +	dispatch_event_fd(&obj_event->obj_sub_list, data);
>> +
>> +	rcu_read_unlock();
>> +	return NOTIFY_OK;
>>   }
>>   
>>   void mlx5_ib_devx_init_event_table(struct mlx5_ib_dev *dev)
>> @@ -2221,7 +2444,7 @@ void mlx5_ib_devx_cleanup_event_table(struct mlx5_ib_dev *dev)
>>   		event = entry;
>>   		list_for_each_entry_safe(sub, tmp, &event->unaffiliated_list,
>>   					 xa_list)
>> -			devx_cleanup_subscription(dev, sub);
>> +			devx_cleanup_subscription(dev, sub, false);
>>   		kfree(entry);
>>   	}
>>   	mutex_unlock(&dev->devx_event_table.event_xa_lock);
>> @@ -2329,18 +2552,126 @@ static const struct file_operations devx_async_cmd_event_fops = {
>>   static ssize_t devx_async_event_read(struct file *filp, char __user *buf,
>>   				     size_t count, loff_t *pos)
>>   {
>> -	return -EINVAL;
>> +	struct devx_async_event_file *ev_file = filp->private_data;
>> +	struct devx_async_event_queue *ev_queue = &ev_file->ev_queue;
>> +	struct devx_event_subscription *event_sub;
>> +	struct devx_async_event_data *uninitialized_var(event);
>> +	int ret = 0;
>> +	size_t eventsz;
>> +	bool omit_data;
>> +	void *event_data;
>> +
>> +	omit_data = ev_queue->flags &
>> +		MLX5_IB_UAPI_DEVX_CREATE_EVENT_CHANNEL_FLAGS_OMIT_EV_DATA;
>> +
>> +	spin_lock_irq(&ev_queue->lock);
>> +
>> +	if (ev_queue->is_overflow_err) {
>> +		ev_queue->is_overflow_err = 0;
>> +		spin_unlock_irq(&ev_queue->lock);
>> +		return -EOVERFLOW;
>> +	}
>> +
>> +	while (list_empty(&ev_queue->event_list)) {
>> +		spin_unlock_irq(&ev_queue->lock);
>> +
>> +		if (filp->f_flags & O_NONBLOCK)
>> +			return -EAGAIN;
>> +
>> +		if (wait_event_interruptible(ev_queue->poll_wait,
>> +			    (!list_empty(&ev_queue->event_list) ||
>> +			     ev_queue->is_destroyed))) {
>> +			return -ERESTARTSYS;
>> +		}
>> +
>> +		if (list_empty(&ev_queue->event_list) &&
>> +		    ev_queue->is_destroyed)
>> +			return -EIO;
> 
> All these tests should be under the lock.

We can't call wait_event_interruptible() above which may sleep under the 
lock, correct ? are you referring to the list_empty() and is_destroyed ?

By the way looking in uverb code [1], similar code which is not done 
under the lock as of here..

[1] 
https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L244


> 
> Why don't we return EIO as soon as is-destroyed happens? What is the
> point of flushing out the accumulated events?

It follows the above uverb code/logic that returns existing events even 
in that case, also the async command events in this file follows that 
logic, I suggest to stay consistent.
> 
>> +
>> +		spin_lock_irq(&ev_queue->lock);
>> +	}
>> +
>> +	if (omit_data) {
>> +		event_sub = list_first_entry(&ev_queue->event_list,
>> +					struct devx_event_subscription,
>> +					event_list);
>> +		eventsz = sizeof(event_sub->cookie);
>> +		event_data = &event_sub->cookie;
>> +	} else {
>> +		event = list_first_entry(&ev_queue->event_list,
>> +				      struct devx_async_event_data, list);
>> +		eventsz = sizeof(struct mlx5_eqe) +
>> +			sizeof(struct mlx5_ib_uapi_devx_async_event_hdr);
>> +		event_data = &event->hdr;
>> +	}
>> +
>> +	if (eventsz > count) {
>> +		spin_unlock_irq(&ev_queue->lock);
>> +		return -ENOSPC;
> 
> This is probably the wrong errno

OK, will change to -EINVAL as in uverbs
https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L254

> 
>> +	}
>> +
>> +	if (omit_data)
>> +		list_del_init(&event_sub->event_list);
>> +	else
>> +		list_del(&event->list);
>> +
>> +	spin_unlock_irq(&ev_queue->lock);
>> +
>> +	if (copy_to_user(buf, event_data, eventsz))
>> +		ret = -EFAULT;
>> +	else
>> +		ret = eventsz;
> 
> This is really poorly ordered, EFAULT will cause the event to be lost. :(

Agree but apparently rare case .. see also the below notes.

> 
> Maybe the event should be re-added on error? Tricky.

What will happen if another copy_to_user may then fail again (loop ?) 
... not sure that we want to get into this tricky handling ...

As of above, It follows the logic from uverbs at that area.
https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_main.c#L267

> 
>> +	if (!omit_data)
>> +		kfree(event);
>> +	return ret;
>>   }
>>   
>>   static __poll_t devx_async_event_poll(struct file *filp,
>>   				      struct poll_table_struct *wait)
>>   {
>> -	return 0;
>> +	struct devx_async_event_file *ev_file = filp->private_data;
>> +	struct devx_async_event_queue *ev_queue = &ev_file->ev_queue;
>> +	__poll_t pollflags = 0;
>> +
>> +	poll_wait(filp, &ev_queue->poll_wait, wait);
>> +
>> +	spin_lock_irq(&ev_queue->lock);
>> +	if (ev_queue->is_destroyed)
>> +		pollflags = EPOLLIN | EPOLLRDNORM | EPOLLRDHUP;
>> +	else if (!list_empty(&ev_queue->event_list))
>> +		pollflags = EPOLLIN | EPOLLRDNORM;
>> +	spin_unlock_irq(&ev_queue->lock);
>> +
>> +	return pollflags;
>>   }
>>   
>>   static int devx_async_event_close(struct inode *inode, struct file *filp)
>>   {
>> +	struct ib_uobject *uobj = filp->private_data;
>> +	struct devx_async_event_file *ev_file =
>> +		container_of(uobj, struct devx_async_event_file, uobj);
>> +	struct devx_event_subscription *event_sub, *event_sub_tmp;
>> +	struct devx_async_event_data *entry, *tmp;
>> +
>> +	mutex_lock(&ev_file->dev->devx_event_table.event_xa_lock);
>> +	/* delete the subscriptions which are related to this FD */
>> +	list_for_each_entry_safe(event_sub, event_sub_tmp,
>> +				 &ev_file->subscribed_events_list, file_list)
>> +		devx_cleanup_subscription(ev_file->dev, event_sub, true);
>> +	mutex_unlock(&ev_file->dev->devx_event_table.event_xa_lock);
>> +
>> +	/* free the pending events allocation */
>> +	if (!(ev_file->ev_queue.flags &
>> +	    MLX5_IB_UAPI_DEVX_CREATE_EVENT_CHANNEL_FLAGS_OMIT_EV_DATA)) {
>> +		spin_lock_irq(&ev_file->ev_queue.lock);
>> +		list_for_each_entry_safe(entry, tmp,
>> +					 &ev_file->ev_queue.event_list, list)
>> +			kfree(entry); /* read can't come any nore */
> 
> spelling

OK

> 
>> +		spin_unlock_irq(&ev_file->ev_queue.lock);
>> +	}
>>   	uverbs_close_fd(filp);
>> +	put_device(&ev_file->dev->ib_dev.dev);
>>   	return 0;
>>   }
>>   
>> @@ -2374,6 +2705,17 @@ static int devx_hot_unplug_async_cmd_event_file(struct ib_uobject *uobj,
>>   static int devx_hot_unplug_async_event_file(struct ib_uobject *uobj,
>>   					    enum rdma_remove_reason why)
>>   {
>> +	struct devx_async_event_file *ev_file =
>> +		container_of(uobj, struct devx_async_event_file,
>> +			     uobj);
>> +	struct devx_async_event_queue *ev_queue = &ev_file->ev_queue;
>> +
>> +	spin_lock_irq(&ev_queue->lock);
>> +	ev_queue->is_destroyed = 1;
>> +	spin_unlock_irq(&ev_queue->lock);
>> +
>> +	if (why == RDMA_REMOVE_DRIVER_REMOVE)
>> +		wake_up_interruptible(&ev_queue->poll_wait);
> 
> Why isn't this wakeup always done?

Maybe you are right and this can be always done to wake up any readers 
as the 'is_destroyed' was set.

By the way, any idea why it was done as such in uverbs [1] for similar 
flow ? also the command events follows that.


[1]
https://elixir.bootlin.com/linux/latest/source/drivers/infiniband/core/uverbs_std_types.c#L207

^ permalink raw reply

* [ANNOUNCE] nftables 0.9.1 release
From: Pablo Neira Ayuso @ 2019-06-24 16:49 UTC (permalink / raw)
  To: netfilter, netfilter-devel; +Cc: netdev

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

Hi!

The Netfilter project proudly presents:

        nftables 0.9.1

This release contains fixes and new features, available up with Linux
kernels >= 5.2.

* IPsec support, which allows matching on IPsec tunnel/beet addresses in xfrm
  state associated with a packet, IPsec request id and the SPI, eg.

        ... ipsec in ip saddr 192.168.1.0/24
        ... ipsec out ip6 daddr @endpoints
        ... ipsec in spi 1-65536

  You can also check if the route performs ipsec tunneling, eg.

        filter output rt ipsec missing drop

  otherwise, drop it.

* IGMP matching support, eg.

        # nft add rule netdev foo bar igmp type membership-query counter drop

  If you want to drop IGMP membership queries from the ingress path.

* Use variable to define jump / goto chain, eg.

        define dest = ber

        add table ip foo
        add chain ip foo bar {type filter hook input priority 0;}
        add chain ip foo ber
        add rule ip foo ber counter
        add rule ip foo bar jump $dest

* Operating System fingerprint (osf) support, eg.

        ... meta mark set osf ttl skip name map { "Linux" : 0x1,
                                                  "Windows" : 0x2,
                                                  "MacOS" : 0x3,
                                                  "unknown" : 0x0 }

  This allows you to mark packets based on the guessed OS. If osf does
  not guess the OS, then traffic falls under the "unknown" OS type. Note
  that the example above skips TTL header field checks.

  You can also check for specific OS version:

        ... osf ttl skip version "Linux:4.20"

  This passive fingerprinting is based on the OS definitions available
  through the pf.os file.

* ARP sender and target IPv4 address matching, eg.

        table arp x {
                chain y {
                        type filter hook input priority filter; policy accept;
                        arp saddr ip 192.168.2.1 counter packets 1 bytes 46
                }
        }

  this updates rule counters for ARP packets originated from the
  192.168.2.1 address.

* transparent proxy support (tproxy), eg.

        table ip x {
                chain y {
                        type filter hook prerouting priority -150; policy accept;
                        tcp dport 80 tproxy to :8080
                }
        }

* socket mark support, to retrieve the socket mark that is set via setsockopt()
  with SO_MARK by the process, eg.

        table inet x {
                chain y {
                        type filter hook prerouting priority -150; policy accept;
                        tcp dport 8080 mark set socket mark
                }
        }

* Support for textual chain priorities, eg.

        nft add table ip x
        nft add chain ip x raw { type filter hook prerouting priority raw; }
        nft add chain ip x filter { type filter hook prerouting priority filter; }
        nft add chain ip x filter_later { type filter hook prerouting priority filter + 10; }

   which are listed in textual priority by default. You can disable this
   via -y option, eg. nft -y list ruleset.

* Secmark support, eg.

        # nft add secmark inet filter sshtag \"system_u:object_r:ssh_server_packet_t:s0\"

  This defines the "sshtag" for this secctx context string, then, you
  can use it from rules to set the secmark:

        # nft add rule inet filter input tcp dport 22 meta secmark set "sshtag"

  you may also combine this with maps:

        # nft add map inet filter secmapping { type inet_service : secmark\; }
        # nft add element inet filter secmapping { 22 : "sshtag" }
        # nft add rule inet filter input meta secmark set tcp dport map @secmapping

* Honor /etc/services, eg.

        # nft add rule x y tcp dport \"ssh\"
        # nft list ruleset -l
        table x {
                chain y {
                        ...
                        tcp dport "ssh"
                }
        }

  You can list this numerically via -S option.

* Interface kind support, eg.

        add rule inet raw prerouting meta iifkind "vrf" accept

  oifkind is also available from the output path.

* Improve support for dynamic set updates, though explicit dynamic flag for
  set updates from the packet path. Syntax has been also updated, eg.

        # cat dynamic-sets.nft
        add table x
        add set x s { type ipv4_addr; size 128; timeout 30s; flags dynamic; }
        add chain x y { type filter hook input priority 0; }
        add rule x y update @s { ip saddr }

  This ruleset updates the set 's' by adding IPv4 source addresses. For
  each packets seen, the timer is refreshed, after 30 seconds of no
  packets seen for this address, this entry expires.

        # nft -f dynamic-sets.nft
        # nft list set x s
        table ip x {
                set s {
                        type ipv4_addr
                        size 128
                        flags dynamic,timeout
                        timeout 30s
                        elements = { 47.215.7.47 expires 26s484ms,
                                     112.212.124.247 expires 25s268ms }
                }
        }

  use this 'dynamic' flag to indicate the kernel that this set will be
  updated from the packet path.

  You can also combine this with stateful expressions, eg.

        table ip x {
                set xyz {
                        type ipv4_addr
                        size 65535
                        flags dynamic,timeout
                        timeout 1h
                }

                chain y {
                        type filter hook output priority filter; policy accept;
                        update @xyz { ip daddr counter } counter
                }
        }

  where each entry in 'xyz' gets a counter.

* Support for connection tracking timeout policies, this allows
  to attach specific timeout policies to flows, eg.

        table ip filter {
                ct timeout agressive-tcp {
                        protocol tcp;
                        l3proto ip;
                        policy = {established: 100, close_wait: 4, close: 4}
                }
                chain output {
                        ...
                        tcp dport 8888 ct timeout set "agressive-tcp"
                }
        }

  that allows you to override the default timeout policy
  (via /proc/sys/net/netfilter/nf_conntrack_*_timeout_* sysctl) for
  packets going to TCP dport 8888.

* NAT support for the inet family, eg.

        table inet nat {
                ...
                ip6 daddr dead::2::1 dnat to dead:2::99
        }

* Improved error reporting through misspell suggestions:

        # nft add table filter
        # nft add chain filtre test
        Error: No such file or directory; did you mean table ‘filter’ in family ip?
        add chain filtre test
                  ^^^^^^

* Print default policy in traces, eg.

        # nft add rule x y meta nftrace set 1
        # nft monitor trace
        trace id 6f2db0af ip x y packet: ...
        trace id 6f2db0af ip x y rule meta nftrace set 1 (verdict continue)
        trace id 6f2db0af ip x y verdict continue
        trace id 6f2db0af ip x y policy accept

* Allow interface names in sets, eg.

        set sc {
               type inet_service . ifname
               elements = { "ssh" . "eth0" }
        }

* Update flowtable rule syntax.

        # nft add table x
        # nft add flowtable x ft { hook ingress priority 0\; devices = { eth0, wlan0 }\; }
        ...
        # nft add rule x forward ip protocol { tcp, udp } flow add @ft

  Prefer 'flow add @ft' for consistency with set and map syntax.

* Improved JSON support.

* Very simple python class which gives access to libnftables API via
  ctypes module.

* A few library documentation updates, see:

        man(3) libnftables
        man(5) libnftables-json

* And memory and file descriptor leak fixes, improved cache logic, among
  many other changes behind the scene...

See ChangeLog that comes attached to this email for more details.

You can download it from:

http://www.netfilter.org/projects/nftables/downloads.html#nftables-0.9.1
ftp://ftp.netfilter.org/pub/nftables/

To build the code, libnftnl 1.1.3 and libmnl >= 1.0.3 are required:

* http://netfilter.org/projects/libnftnl/index.html
* http://netfilter.org/projects/libmnl/index.html

Visit our wikipage for user documentation at:

* http://wiki.nftables.org

For the manpage reference, check man(8) nft.

In case of bugs and feature request, file them via:

* https://bugzilla.netfilter.org

Happy firewalling!

[-- Attachment #2: changes-nftables-0.9.1.txt --]
[-- Type: text/plain, Size: 21030 bytes --]

Arturo Borrero Gonzalez (1):
      tests: fix return codes

Arushi Singhal (6):
      nftables: Fix typos/Grammatical Errors
      nftables: tests: shell: Replace "%" with "#" or "$"
      nft: doc: Convert man page source to asciidoc
      doc: correct some typos in asciidoc
      nft: doc: fix typos in asciidoc
      nft: doc: fix make distcheck

Christian Göttsche (1):
      src: add support for setting secmark

Duncan Roe (10):
      doc: Remove UTF8(?) sequences
      doc: resolve run-together IPv6 address specification headers
      doc: Miscellaneous spelling fixes
      doc: Changes following detailed comparison with last XML version
      doc: user niggles
      doc: Remove double-spacing in text
      doc: Add script to build PDF files
      rule: Fix build failure in rule.c
      doc: Re-work RULES:add/insert/replace to read better.
      doc: libnftables.adoc misc cleanups

Eric Garver (5):
      parser_json: default to unspecified l3proto for ct helper/timeout
      parser_json: fix off by one index on rule add/replace
      parser_json: fix crash on add rule to bad references
      py: fix missing decode/encode of strings
      src: update cache if cmd is more specific

Eric Leblond (8):
      configure.ac: better message when a2x is missing
      configure.ac: remove useless braces in messages
      configure.ac: docbook2man invalid syntax error
      python: installation of binding via make install
      python: set license and author in nftables.py
      doc: fix make distcheck
      tests/py: minor cleaning
      tests/py: fix import when run from other directory

Fernando Fernandez Mancera (23):
      src: fix a typo in socket.h
      src: introduce passive OS fingerprint matching
      tests: py: add test cases for "osf" matching
      doc: add osf expression to man page
      test: py: fix osf testcases warning
      src: use NFT_OSF_MAXGENRELEN instead of IFNAMSIZ in osf.c
      tests: improve test cases for osf
      files: osf: copy iptables/utils/pf.os into nftables tree
      src: mnl: make nft_mnl_talk() public
      src: osf: import nfnl_osf.c to load osf fingerprints
      src: osf: load pf.os from expr_evaluate_osf()
      include: add missing xfrm.h to Makefile.am
      osf: add ttl option support
      doc: osf: add ttl option to man page
      doc: update nft list plural form parameters
      osf: add version fingerprint support
      json: osf: add version json support
      tests: py: add osf tests with versions
      doc: add osf version option to man page
      files: osf: update pf.os with newer OS fingerprints
      files: pf.os: merge the signatures splitted by version
      src: Introduce chain_expr in jump and goto statements
      src: Allow goto and jump to a variable

Florian Westphal (52):
      datatype: add stolen verdict
      src: trace: fix policy printing
      rule: limit: don't print default burst value
      doc: describe dynamic flag and caveats for packet-path updates
      nft: set: print dynamic flag when set
      tests: check ifname use in concatenated sets
      tests: add test case for rename-to-same-name
      src: meta: always prefix 'meta' for almost all tokens
      doc: remove nft.xml from CLEANFILES
      parser: avoid nf_key_proto redefinitions
      src: osf: add json support
      src: tproxy: relax family restrictions
      src: tproxy: add json support
      tests: fix json output for osf, socket and tproxy expressions
      proto: fix icmp/icmpv6 code datatype
      evaluate: throw distinct error if map exists but contains no objects
      src: rt: add support to check if route will perform ipsec transformation
      src: rename meta secpath to meta ipsec
      documentation: clarify iif vs. iifname
      xt: pass octx to translate function
      xt: always build with a minimal support for xt match/target decode
      tests: add test case for rule replacement expression deactivation
      xt: fix build when libxtables is not installed
      xt: fix build with --with-xtables
      rule: fix object listing when no table is given
      tests: shell: add test case for leaking of stateful object refcount
      tests: shell: change all test scripts to return 0
      tests: shell: fix up redefine test case
      tests: shell: remove RETURNCODE_SEPARATOR
      src: fix netdev family device name parsing
      payload: refine payload expr merging
      mnl: name is ignored when deleting a table
      doc: fix non-working example
      tests: fix up expected payloads after expr merge change
      src: expr: add and use expr_name helper
      src: payload: export and use payload_expr_cmp
      src: expr: add and use internal expr_ops helper
      src: expr: add expression etype
      src: expr: remove expr_ops from struct expr
      src: expr: fix build failure with json support
      doc: update goto/jump help text
      segtree: fix crash when debug mode is active
      tests: add test case for anon set abort.
      src: add nat support for the inet family
      src: fix double free on xt stmt destruction
      tests: shell: avoid single-value anon sets
      tests: py: remove single-value-anon-set test cases
      datatype: fix print of raw numerical symbol values
      tests: add missing json arp operation output
      netlink_delinerize: remove network header dep for reject statement also in bridge family
      src: statement: disable reject statement type omission for bridge
      src: prefer meta protocol as bridge l3 dependency

Harsha Sharma (6):
      rule: list only the table containing object
      tests: shell: add tests for listing objects
      src: add ct timeout support
      tests: py: add ct timeout tests
      tests: shell: add tests for ct timeout objects
      doc: Document ct timeout support

Jan Engelhardt (1):
      doc: grammar fixes

Laura Garcia Liebana (2):
      json: fix json_events_cb() declaration when libjansson is not present
      parser_json: fix segfault in translating string to nft object

Loganaden Velvindron (1):
      proto: support for draft-ietf-tsvwg-le-phb-10.txt

Luis Ressel (2):
      configure.ac: Fix a2x check
      configure.ac: Clean up AC_ARG_{WITH, ENABLE} invocations, s/==/=/

Máté Eckl (18):
      doc: Add socket expression to man page
      doc: nft.txt: Wrap extra long lines to 80 chars
      doc: data-types.txt: Wrap extra long lines to 80 chars
      doc: payload-expression.txt: Wrap extra long lines to 80 chars
      doc: primary-expression.txt: Wrap extra long lines to 80 chars
      doc: stateful-objects.txt: Wrap extra long lines to 80 chars
      doc: statements.txt: Wrap extra long lines to 80 chars
      src: Add tproxy support
      tests: py: Add test cases for tproxy support
      doc: Add tproxy statement to man page
      src: Expose socket mark via socket expression
      doc: fix syntax for RULES
      doc: Add comment possibility to man page
      src: Set/print standard chain prios with textual names
      src: Make invalid chain priority error more specific
      test: shell: Test cases for standard chain prios
      test: shell: Test cases for standard prios for flowtables
      src: add ipsec (xfrm) expression

Pablo Neira Ayuso (145):
      tests: build: cover --with-json too
      src: add dynamic flag and use it
      src: add --literal option
      doc: update manpage to document --literal option
      evaluate: skip evaluation of datatype concatenations
      tests: shell: validate maximum chain depth
      include: add missing osf.h
      parser_bison: allow to use new osf expression from assignment statement
      tests: py: test osf with sets
      tests: shell: validate too deep jumpstack from basechain
      tests: shell: fix 0012different_defines_0 with meta mark
      tests: shell: missing modules in cleanup path
      build: remove PDF documentation generation
      statement: incorrect spacing in set reference
      rule: do not print elements in dynamically populated sets with `-s'
      src: simplify map statement
      src: integrate stateful expressions into sets and maps
      src: honor /etc/services
      tests: build: no need for root to run build tests
      tests: build: run make distcheck from fresh clone
      tests: build: run make on each ./configure option
      tests: shell: missing dump for 0017ct_timeout_obj_0
      nfnl_osf: display debugging information from --debug=mnl
      segtree: bogus range via get set element on existing elements
      segtree: disantangle get_set_interval_end()
      segtree: memleak in get_set_decompose()
      rule: fix memleak in do_get_setelems()
      segtree: stop iteration on existing elements in case range is found
      netlink: remove markup json parsing code
      src: get rid of netlink_genid_get()
      mnl: remove alloc_nftnl_table()
      mnl: remove alloc_nftnl_chain()
      mnl: remove alloc_nftnl_rule()
      mnl: remove alloc_nftnl_set()
      src: remove netlink_flush_table()
      src: remove netlink_flush_chain()
      segtree: incorrect handling of last element in get_set_decompose()
      segtree: set proper error cause on existing elements
      src: remove opts field from struct xt_stmt
      evaluate: bogus bail out with raw expression from dynamic sets
      src: pass struct nft_ctx through struct eval_ctx
      src: pass struct nft_ctx through struct netlink_ctx
      netlink: reset mnl_socket field in struct nft_ctx on EINTR
      src: move socket open and reopen to mnl.c
      mnl: remove alloc_nftnl_obj()
      mnl: use either name or handle to refer to objects
      mnl: remove alloc_nftnl_flowtable()
      netlink: remove netlink_batch_send()
      evaluate: do not pass EXPR_SET_ELEM to stmt_evaluate_arg() for set/map evaluation
      evaluate: stmt_evaluate_map() needs right hand side evaluation too
      src: Revert --literal, add -S/--service
      src: add nft_ctx_output_{get,set}_stateless() to nft_ctx_output_{get,flags}_flags
      src: add nft_ctx_output_{get,set}_handle() to nft_ctx_output_{get,set}_flags
      src: add nft_ctx_output_{get,set}_json() to nft_ctx_output_{get,set}_flags
      src: add nft_ctx_output_{get,set}_echo() to nft_ctx_output_{get,set}_flags
      src: default to numeric UID and GID listing
      src: add NFT_CTX_OUTPUT_NUMERIC_PROTO
      src: add -y to priority base chain nummerically
      src: get rid of nft_ctx_output_{get,set}_numeric()
      src: add -p to print layer 4 protocol numerically
      expression: always print range expression numerically
      doc: remove unnecessary extra asterisk at the end of option line
      src: introduce simple hints on incorrect table
      src: introduce simple hints on incorrect chain
      src: introduce simple hints on incorrect set
      utils: remove type checks in min() and max()
      src: provide suggestion for misspelled object name
      misspell: add distance threshold for suggestions
      src: introduce simple hints on incorrect object
      src: introduce simple hints on incorrect identifier
      doc: nft: document ct count
      parser: bail out on incorrect burst unit
      src: remove deprecated code for export/import commands
      doc: refer to meta protocol in icmp and icmpv6
      src: add igmp support
      include: add cplusplus guards for extern
      tests: shell: exercise abort path with anonymous set that is bound to rule
      tests: shell: flush after rule deletion
      segtree: remove dummy debug_octx
      segtree: add missing non-matching segment to set in flat representation
      evaluate: misleading error reporting with sets and maps
      tests: shell: bogus EBUSY in set deletion after flush
      tests: shell: bogus ENOENT on element deletion in interval set
      tests: shell: bogus EBUSY on helper deletion from transaction
      parser_bison: no need for statement separator for ct object commands
      src: file descriptor leak in include_file()
      build: missing misspell.h in Makefile.am
      src: use 'flow add' syntax
      evaluate: skip binary transfer for named sets
      parser_bison: missing tproxy syntax with port only for inet family
      evaluate: improve error reporting in tproxy with inet family
      ct: use nft_print() instead of printf()
      parser_bison: type_identifier string memleak
      src: missing destroy function in statement definitions
      src: memleak in expressions
      segtree: fix memleak in interval_map_decompose()
      Revert "proto: support for draft-ietf-tsvwg-le-phb-10.txt"
      include: refresh nf_tables.h cached copy
      src: use definitions in include/linux/netfilter/nf_tables.h
      include: refresh nf_tables.h cached copy
      Revert "tests: py: remove single-value-anon-set test cases"
      Revert "tests: shell: avoid single-value anon sets"
      src: support for arp sender and target ethernet and IPv4 addresses
      src: add cache_is_complete() and cache_is_updated()
      tests: replace single element sets
      mnl: add mnl_set_rcvbuffer() and use it
      mnl: mnl_set_rcvbuffer() skips buffer size update if it is too small
      mnl: call mnl_set_sndbuffer() from mnl_batch_talk()
      mnl: add mnl_nft_batch_to_msg()
      mnl: estimate receiver buffer size
      mnl: mnl_batch_talk() returns -1 on internal netlink errors
      erec: remove double \n on error when internal_netlink is used
      src: dynamic input_descriptor allocation
      src: perform evaluation after parsing
      src: Display parser and evaluate errors in one shot
      src: single cache_update() call to build cache before evaluation
      src: generation ID is 32-bit long
      rule: ensure cache consistency
      evaluate: use-after-free in implicit set
      libnftables: keep evaluating until parser_max_errors
      mnl: bogus error when running monitor mode
      libnftables: check for errors after evaluations
      src: invalid read when importing chain name
      src: invalid read when importing chain name (trace and json)
      expression: use expr_clone() from verdict_expr_clone()
      netlink_delinearize: release expressions in context registers
      netlink_delinearize: release expression before calling netlink_parse_concat_expr()
      parser_bison: free chain name after creating constant expression
      src: add reference counter for dynamic datatypes
      datatype: dtype_clone() should clone flags too
      netlink_delinearize: use-after-free in expr_postprocess_string()
      evaluate: use-after-free in meter
      evaluate: update byteorder only for implicit maps
      evaluate: double datatype_free() with dynamic integer datatypes
      cache: do not populate the cache in case of flush ruleset command
      src: remove useless parameter from cache_flush()
      tests: shell: cannot use handle for non-existing rule in kernel
      rule: skip cache population from do_command_monitor()
      netlink: remove netlink_list_table()
      src: add cache level flags
      evaluate: allow get/list/flush dynamic sets and maps via list command
      evaluate: do not allow to list/flush anonymous sets via list command
      rule: do not suggest anonymous sets on mispelling errors
      ct: support for NFT_CT_{SRC,DST}_{IP,IP6}
      build: Bump version to v0.9.1

Phil Sutter (112):
      JSON: Call verdict maps 'vmap' in JSON as well
      tests/py: Fix JSON for flowtable tests
      JSON: Don't print burst if equal to 5
      JSON: Add support for socket expression
      JSON: Add support for connlimit statement
      JSON: Support latest enhancements of fwd statement
      doc: Add JSON schema documentation
      doc: Add libnftables man page
      doc: Fix typo in Makefile.am
      libnftables: Fix exit_cookie()
      libnftables: Simplify nft_run_cmd_from_buffer footprint
      scanner: Do not convert tabs into spaces
      doc: libnftables-json: Review asciidoc syntax
      Makefile: Introduce Make_global.am
      netlink_delinearize: Refactor meta_may_dependency_kill()
      evaluate: reject: Allow icmpx in inet/bridge families
      json: Fix compile error
      tests: py: Fix coloring of differences
      doc: Document implicit dependency creation for icmp/icmpv6
      doc: Improve example in libnftables-json(5)
      doc: Review libnftables-json.adoc
      JSON: Make meta statement/expression extensible
      JSON: Review verdict statement and expression
      JSON: Review payload expression
      JSON: Rename (v)map expression properties
      JSON: Rename mangle statement properties
      JSON: Make match op mandatory, introduce 'in' operator
      JSON: Add metainfo object to all output
      py: trivial: Fix typo in comment string
      parser_json: Fix crash in error reporting
      tests/py: Make nft-test.py a little more robust
      src: Fix literal check for inet_service type
      tests/py: Check differing rule output for sanity
      json: Fix datatype_json() for literal level
      json: Make inet_service_type_json() respect literal level
      json: Print range expressions numerically
      tests/py: Fix JSON for icmp*.t
      nft.8: Update meta pkt_type value description
      doc: Review man page building in Makefile.am
      parser_bison: Fix for chain prio name 'out'
      tests: shell: Fix indenting in 0021prio_0
      tests: shell: Drop one-time use variables in 0021prio_0
      tests: shell: Improve gen_chains() in 0021prio_0
      tests: shell: Improve performance of 0021prio_0
      tests: shell: Test 'get element' command
      parser_bison: Fix for ECN keyword in LHS of relational
      tests/py: Add missing JSON bits for inet/meta.t
      json: Drop unused symbolic_constant_json() stub
      json: Add ct timeout support
      monitor: Drop fake XML support
      monitor: Drop 'update table' and 'update chain' cases
      monitor: Fix printing of ct objects
      monitor: Use libnftables JSON output
      tests: monitor: Test JSON output as well
      Fix memleak in netlink_parse_fwd() error path
      libnftables: Fix memleak in nft_parse_bison_filename()
      parser_json: Fix for ineffective family value checks
      json: Fix memleak in dup_stmt_json()
      tests: shell: Extend get element test
      include: Fix comment for struct eval_ctx
      json: Fix osf ttl support
      json: Fix for recent changes to context structs
      mnl: Improve error checking in mnl_nft_event_listener()
      json: Work around segfault when encountering xt stmt
      tests/shell: Add testcase for cache update problems
      JSON: Add support for echo option
      nft.8: Document log level audit
      py: Adjust Nftables class to output flags changes
      doc: Fix for make distcheck
      nft.8: Clarify 'index' option of add rule command
      src: Reject 'export vm json' command
      libnftables: Print errors before freeing commands
      parser_json: Duplicate chain name when parsing jump verdict
      parser_json: Use xstrdup() when parsing rule comment
      json: Fix memleaks in echo support
      parser_json: Respect base chain priority
      parser_json: Rewrite echo support
      doc: Add minimal description of (v)map statements
      parser_json: Disallow ct helper as type to map to
      tests: monitor: Adjust to changed events ordering
      tests/py: Fix error messages in chain_delete()
      parser_json: Fix typo in ct timeout policy parser
      parser_json: Fix parser for list maps command
      src: use UDATA defines from libnftnl
      py: Fix gitignore of lib/ directory
      doc: Review man page synopses
      json: Support nat in inet family
      parser_json: Fix igmp support
      netlink: Fix printing of zero-length prefixes
      tests/py: Fix JSON equivalents of osf tests
      json: Fix tproxy support regarding latest changes
      parser_json: Fix ct timeout object support
      tests/py: Fix JSON expected output after expr merge change
      tests/py: Fix JSON expected output for icmpv6 code values
      parser_json: Fix and simplify verdict expression parsing
      tests/shell: Test large transaction with echo output
      mnl: Initialize fd_set before select(), not after
      mnl: Simplify mnl_batch_talk()
      py: Implement JSON validation in nftables module
      tests/py: Support JSON validation
      src: Fix cache_flush() in cache_needs_more() logic
      libnftables: Drop cache in error case
      cache: Fix evaluation for rules with index reference
      tests/json_echo: Drop needless workaround
      rule: Introduce rule_lookup_by_index()
      src: Make cache_is_complete() public
      src: Support intra-transaction rule references
      tests/py: Fix JSON equivalents
      tests/py: Add missing arp.t JSON equivalents
      tests/shell: Fix warning from awk call
      tests/shell: Print unified diffs in dump errors
      monitor: Accept -j flag

Rosen Penev (1):
      gmputil: Add missing header for va_list

Shekhar Sharma (1):
      tests: json_echo: convert to py3

Ville Skyttä (1):
      doc: Spelling and grammar fixes

wenxu (1):
      meta: add iifkind and oifkind support


^ permalink raw reply

* Re: Removing skb_orphan() from ip_rcv_core()
From: Eric Dumazet @ 2019-06-24 16:49 UTC (permalink / raw)
  To: Jamal Hadi Salim, Joe Stringer, Eric Dumazet, Florian Westphal
  Cc: netdev, john fastabend, Daniel Borkmann, Lorenz Bauer,
	Jakub Sitnicki, Paolo Abeni
In-Reply-To: <ab745372-35eb-8bb8-30a4-0e861af27ac2@mojatatu.com>



On 6/24/19 7:47 AM, Jamal Hadi Salim wrote:
> On 2019-06-21 1:58 p.m., Joe Stringer wrote:
>> Hi folks, picking this up again..
> [..]
>> During LSFMM, it seemed like no-one knew quite why the skb_orphan() is
>> necessary in that path in the current version of the code, and that we
>> may be able to remove it. Florian, I know you weren't in the room for
>> that discussion, so raising it again now with a stack trace, Do you
>> have some sense what's going on here and whether there's a path
>> towards removing it from this path or allowing the skb->sk to be
>> retained during ip_rcv() in some conditions?
> 
> 
> Sorry - I havent followed the discussion but saw your email over
> the weekend and wanted to be at work to refresh my memory on some
> code. For maybe 2-3 years we have deployed the tproxy
> equivalent as a tc action on ingress (with no netfilter dependency).
> 
> And, of course, we had to work around that specific code you are
> referring to - we didnt remove it. The tc action code increments
> the sk refcount and sets the tc index. The net core doesnt orphan
> the skb if a speacial tc index value is set (see attached patch)
> 
> I never bothered up streaming the patch because the hack is a bit embarrassing (but worked ;->); and never posted the action code
> either because i thought this was just us that had this requirement.
> I am glad other people see the need for this feature. Is there effort
> to make this _not_ depend on iptables/netfilter? I am guessing if you
> want to do this from ebpf (tc or xdp) that is a requirement.
> Our need was with tcp at the time; so left udp dependency on netfilter
> alone.
> 


Well, I would simply remove the skb_orphan() call completely.



^ permalink raw reply

* Re: [RFC PATCH bpf-next] RV32G eBPF JIT
From: Jiong Wang @ 2019-06-24 16:45 UTC (permalink / raw)
  To: Luke Nelson
  Cc: Luke Nelson, Xi Wang, Palmer Dabbelt, Albert Ou,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, Björn Töpel, linux-riscv, linux-kernel,
	netdev, bpf
In-Reply-To: <20190621225938.27030-1-lukenels@cs.washington.edu>


Luke Nelson writes:

> From: Luke Nelson <luke.r.nels@gmail.com>
>
> This is an eBPF JIT for RV32G, adapted from the JIT for RV64G.
> Any feedback would be greatly appreciated.
>
> It passes 359 out of 378 tests in test_bpf.ko. The failing tests are
> features that are not supported right now:
>   - ALU64 DIV/MOD:
>       These require loops to emulate on 32-bit hardware,
>       and are not supported on other 32-bit JITs like
>       ARM32.
>   - BPF_XADD | BPF_DW:
>       RV32G does not have atomic instructions for operating
>       on double words. This is similar to ARM32.
>   - Tail calls:
>       I'm working on adding support for these now, but couldn't
>       find any test cases that use them. What's the best way
>       of testing tail call code?
>   - Far branches
>       These are not supported in RV64G either.
>
> There are two main changes required for this to work compared to the
> RV64 JIT.
>
> First, eBPF registers are 64-bit, while RV32G registers are 32-bit.
> I take an approach similar to ARM32: most BPF registers map directly to
> 2 RISC-V registers, while some reside in stack scratch space and must
> be saved / restored when used.
>
> Second, many 64-bit ALU operations do not trivially map to 32-bit
> operations. Operations that move bits between high and low words, such
> as ADD, LSH, MUL, and others must emulate the 64-bit behavior in terms
> of 32-bit instructions.
>
> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
> Cc: Xi Wang <xi.wang@gmail.com>
> ---
>  arch/riscv/Kconfig              |    2 +-
>  arch/riscv/net/Makefile         |    7 +-
>  arch/riscv/net/bpf_jit_comp32.c | 1460 +++++++++++++++++++++++++++++++
>  3 files changed, 1467 insertions(+), 2 deletions(-)
>  create mode 100644 arch/riscv/net/bpf_jit_comp32.c
>
<snip>
> +static void rv32_bpf_put_reg32(const s8 *reg, const s8 *src,
> +			       struct rv_jit_context *ctx)
> +{
> +	if (is_stacked(reg[1])) {
> +		emit(rv_sw(RV_REG_FP, reg[1], src[1]), ctx);
> +		emit(rv_sw(RV_REG_FP, reg[0], RV_REG_ZERO), ctx);
> +	} else {
> +		emit(rv_addi(reg[0], RV_REG_ZERO, 0), ctx);
> +	}
> +}
> +

Looks to me 32-bit optimization is not enabled.

If you define bpf_jit_needs_zext to return true

  bool bpf_jit_needs_zext(void)
  {
        return true;
  }

Then you don't need to zero high 32-bit when writing 32-bit sub-register
and you just need to implement the explicit zero extension insn which is a
special variant of BPF_MOV. This can save quite a few instructions. RV64
and arches like arm has implemented this, please search
"aux->verifier_zext".

And there is a doc for this optimization:

https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/Documentation/bpf/bpf_design_QA.rst#n168

Regards,
Jiong

^ permalink raw reply

* Re: [PATCH net v2 1/2] ipv6: constify rt6_nexthop()
From: Nick Desaulniers @ 2019-06-24 16:45 UTC (permalink / raw)
  To: Nicolas Dichtel; +Cc: David S. Miller, netdev, kbuild test robot
In-Reply-To: <20190624140109.14775-2-nicolas.dichtel@6wind.com>

On Mon, Jun 24, 2019 at 7:01 AM Nicolas Dichtel
<nicolas.dichtel@6wind.com> wrote:
>
> There is no functional change in this patch, it only prepares the next one.
>
> rt6_nexthop() will be used by ip6_dst_lookup_neigh(), which uses const
> variables.
>
> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>

Also, I think this fixes an issues reported by 0day:
https://groups.google.com/forum/#!searchin/clang-built-linux/const%7Csort:date/clang-built-linux/umkS84jS9m8/GAVVEgNYBgAJ

Reported-by: kbuild test robot <lkp@intel.com>
Acked-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/net/vrf.c                | 2 +-
>  include/net/ip6_route.h          | 4 ++--
>  net/bluetooth/6lowpan.c          | 4 ++--
>  net/ipv6/ip6_output.c            | 2 +-
>  net/netfilter/nf_flow_table_ip.c | 2 +-
>  5 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c
> index 11b9525dff27..311b0cc6eb98 100644
> --- a/drivers/net/vrf.c
> +++ b/drivers/net/vrf.c
> @@ -350,8 +350,8 @@ static int vrf_finish_output6(struct net *net, struct sock *sk,
>  {
>         struct dst_entry *dst = skb_dst(skb);
>         struct net_device *dev = dst->dev;
> +       const struct in6_addr *nexthop;
>         struct neighbour *neigh;
> -       struct in6_addr *nexthop;
>         int ret;
>
>         nf_reset(skb);
> diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
> index 4790beaa86e0..ee7405e759ba 100644
> --- a/include/net/ip6_route.h
> +++ b/include/net/ip6_route.h
> @@ -262,8 +262,8 @@ static inline bool ip6_sk_ignore_df(const struct sock *sk)
>                inet6_sk(sk)->pmtudisc == IPV6_PMTUDISC_OMIT;
>  }
>
> -static inline struct in6_addr *rt6_nexthop(struct rt6_info *rt,
> -                                          struct in6_addr *daddr)
> +static inline const struct in6_addr *rt6_nexthop(const struct rt6_info *rt,
> +                                                const struct in6_addr *daddr)
>  {
>         if (rt->rt6i_flags & RTF_GATEWAY)
>                 return &rt->rt6i_gateway;
> diff --git a/net/bluetooth/6lowpan.c b/net/bluetooth/6lowpan.c
> index 19d27bee285e..1555b0c6f7ec 100644
> --- a/net/bluetooth/6lowpan.c
> +++ b/net/bluetooth/6lowpan.c
> @@ -160,10 +160,10 @@ static inline struct lowpan_peer *peer_lookup_dst(struct lowpan_btle_dev *dev,
>                                                   struct in6_addr *daddr,
>                                                   struct sk_buff *skb)
>  {
> -       struct lowpan_peer *peer;
> -       struct in6_addr *nexthop;
>         struct rt6_info *rt = (struct rt6_info *)skb_dst(skb);
>         int count = atomic_read(&dev->peer_count);
> +       const struct in6_addr *nexthop;
> +       struct lowpan_peer *peer;

I see the added const, but I'm not sure why the declarations were
reordered?  Here and below. Doesn't matter for code review (doesn't
necessitate a v2).

>
>         BT_DBG("peers %d addr %pI6c rt %p", count, daddr, rt);
>
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 834475717110..21efcd02f337 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -59,8 +59,8 @@ static int ip6_finish_output2(struct net *net, struct sock *sk, struct sk_buff *
>  {
>         struct dst_entry *dst = skb_dst(skb);
>         struct net_device *dev = dst->dev;
> +       const struct in6_addr *nexthop;
>         struct neighbour *neigh;
> -       struct in6_addr *nexthop;
>         int ret;
>
>         if (ipv6_addr_is_multicast(&ipv6_hdr(skb)->daddr)) {
> diff --git a/net/netfilter/nf_flow_table_ip.c b/net/netfilter/nf_flow_table_ip.c
> index 241317473114..cdfc33517e85 100644
> --- a/net/netfilter/nf_flow_table_ip.c
> +++ b/net/netfilter/nf_flow_table_ip.c
> @@ -439,9 +439,9 @@ nf_flow_offload_ipv6_hook(void *priv, struct sk_buff *skb,
>         struct nf_flowtable *flow_table = priv;
>         struct flow_offload_tuple tuple = {};
>         enum flow_offload_tuple_dir dir;
> +       const struct in6_addr *nexthop;
>         struct flow_offload *flow;
>         struct net_device *outdev;
> -       struct in6_addr *nexthop;
>         struct ipv6hdr *ip6h;
>         struct rt6_info *rt;
>
> --
> 2.21.0
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply

* [PATCH net,stable] qmi_wwan: Fix out-of-bounds read
From: Bjørn Mork @ 2019-06-24 16:45 UTC (permalink / raw)
  To: netdev; +Cc: linux-usb, Hillf Danton, Bjørn Mork, Kristian Evensen

The syzbot reported

 Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  print_address_description+0x67/0x231 mm/kasan/report.c:188
  __kasan_report.cold+0x1a/0x32 mm/kasan/report.c:317
  kasan_report+0xe/0x20 mm/kasan/common.c:614
  qmi_wwan_probe+0x342/0x360 drivers/net/usb/qmi_wwan.c:1417
  usb_probe_interface+0x305/0x7a0 drivers/usb/core/driver.c:361
  really_probe+0x281/0x660 drivers/base/dd.c:509
  driver_probe_device+0x104/0x210 drivers/base/dd.c:670
  __device_attach_driver+0x1c2/0x220 drivers/base/dd.c:777
  bus_for_each_drv+0x15c/0x1e0 drivers/base/bus.c:454

Caused by too many confusing indirections and casts.
id->driver_info is a pointer stored in a long.  We want the
pointer here, not the address of it.

Thanks-to: Hillf Danton <hdanton@sina.com>
Reported-by: syzbot+b68605d7fadd21510de1@syzkaller.appspotmail.com
Cc: Kristian Evensen <kristian.evensen@gmail.com>
Fixes: e4bf63482c30 ("qmi_wwan: Add quirk for Quectel dynamic config")
Signed-off-by: Bjørn Mork <bjorn@mork.no>
---
The bug was introduced in v5.2-rc1 but has been backported to stable kernels.
So this fix also needs to go into stable.

 drivers/net/usb/qmi_wwan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/usb/qmi_wwan.c b/drivers/net/usb/qmi_wwan.c
index d080f8048e52..8b4ad10cf940 100644
--- a/drivers/net/usb/qmi_wwan.c
+++ b/drivers/net/usb/qmi_wwan.c
@@ -1482,7 +1482,7 @@ static int qmi_wwan_probe(struct usb_interface *intf,
 	 * different. Ignore the current interface if the number of endpoints
 	 * equals the number for the diag interface (two).
 	 */
-	info = (void *)&id->driver_info;
+	info = (void *)id->driver_info;
 
 	if (info->data & QMI_WWAN_QUIRK_QUECTEL_DYNCFG) {
 		if (desc->bNumEndpoints == 2)
-- 
2.11.0


^ 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