* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: David Miller @ 2017-10-21 11:16 UTC (permalink / raw)
To: lucien.xin; +Cc: eric.dumazet, edumazet, netdev, marcelo.leitner, sd
In-Reply-To: <CADvbK_cLxOGpBwdSX+CkE+HLefT4rjz8tbgrkosGg7f+wmQVTA@mail.gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 21 Oct 2017 17:45:09 +0800
> Let's just see if David could accept the patches if I will
> remove the "security claim" from changelog, considering
> it as an improvement of sock diag.
>
> David ?
No I won't. See my other response.
Use modules.conf or netfilter rules to block "scary networking
protocols" if you are so paranoid about this happening.
Thank you.
^ permalink raw reply
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: David Miller @ 2017-10-21 11:14 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, edumazet, marcelo.leitner, sd
In-Reply-To: <CADvbK_fWmmC3ggpoT--Pxk3GxZ8Gq_rbdFGTuXk-BuTHTO=eXw@mail.gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Sat, 21 Oct 2017 14:06:27 +0800
> Imagine a customer generates a sosreport on their system, and
> with that, it loads sctp module. From then on, if their firewall
> doesn't block incoming packets for sctp, they may be prone to some
> remotely triggerable issue on sctp code, without even actually using
> sctp.
Like I said, if the protocol is so unsafe, block it in the
modules.conf file.
Block all "I don't use this" protocols in netfilter.
Otherwise, like I said, any user on their system can open a socket of
the indicated protocol.
There are many options.
Furthermore, "ss" should not signal an error because the protocol
module happens to not be open yet and as I understand it this is what
your patch does since it chooses to not load the module in this
situation.
^ permalink raw reply
* Re: [PATCH v3 1/4] phylib: Add device reset GPIO support
From: Geert Uytterhoeven @ 2017-10-21 10:08 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Florian Fainelli, Geert Uytterhoeven, David S . Miller,
Andrew Lunn, Simon Horman, Magnus Damm, Rob Herring, Mark Rutland,
Nicolas Ferre, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-Renesas,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <44bc181e-6fd2-4868-3c8d-64e66d9c8d6b-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
On Sat, Oct 21, 2017 at 12:03 PM, Sergei Shtylyov
<sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org> wrote:
> On 10/20/2017 9:11 PM, Florian Fainelli wrote:
>>> From: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
>>> --- a/drivers/of/of_mdio.c
>>> +++ b/drivers/of/of_mdio.c
>
> [...]
>>>
>>> @@ -55,10 +56,22 @@ static int of_mdiobus_register_phy(struct mii_bus
>>> *mdio,
>>> is_c45 = of_device_is_compatible(child,
>>> "ethernet-phy-ieee802.3-c45");
>>> + /* Deassert the optional reset signal */
>>> + gpiod = fwnode_get_named_gpiod(&child->fwnode, "reset-gpios", 0,
>>> + GPIOD_OUT_LOW, "PHY reset");
>>> + if (PTR_ERR(gpiod) == -ENOENT)
>>> + gpiod = NULL;
>>> + else if (IS_ERR(gpiod))
>>> + return PTR_ERR(gpiod);
>>> +
>>> if (!is_c45 && !of_get_phy_id(child, &phy_id))
>>> phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
>>> else
>>> phy = get_phy_device(mdio, addr, is_c45);
>>> +
>>> + /* Assert the reset signal again */
>>> + gpiod_set_value(gpiod, 1);
>>
>> You have a phy_device reference now, so why not call phy_device_reset()
>> directly here?
>
> Symmetry, perhaps? (There was a gpiod_set_value(gpiod, 0) call above
> it...
Not only because of symmetry: the validity of the phy object hasn't been
checked yet (that's done immediately below), and phy->mdio.reset hasn't been
filled in yet.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH 2/2] slab, slub, slob: convert slab_flags_t to 32-bit
From: Alexey Dobriyan @ 2017-10-21 10:06 UTC (permalink / raw)
To: akpm
Cc: linux-mm, cl, penberg, rientjes, iamjoonsoo.kim, ecryptfs,
linux-xfs, kasan-dev, netdev
In-Reply-To: <20171021100225.GA22428@avx2>
struct kmem_cache::flags is "unsigned long" which is unnecessary on
64-bit as no flags are defined in the higher bits.
Switch the field to 32-bit and save some space on x86_64 until such
flags appear:
add/remove: 0/0 grow/shrink: 0/107 up/down: 0/-657 (-657)
function old new delta
sysfs_slab_add 720 719 -1
...
check_object 699 676 -23
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
include/linux/slab.h | 44 ++++++++++++++++++++++----------------------
include/linux/types.h | 2 +-
mm/slab.c | 4 ++--
mm/slub.c | 4 ++--
4 files changed, 27 insertions(+), 27 deletions(-)
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -21,19 +21,19 @@
* The ones marked DEBUG are only valid if CONFIG_DEBUG_SLAB is set.
*/
/* DEBUG: Perform (expensive) checks on alloc/free */
-#define SLAB_CONSISTENCY_CHECKS ((slab_flags_t __force)0x00000100UL)
+#define SLAB_CONSISTENCY_CHECKS ((slab_flags_t __force)0x00000100U)
/* DEBUG: Red zone objs in a cache */
-#define SLAB_RED_ZONE ((slab_flags_t __force)0x00000400UL)
+#define SLAB_RED_ZONE ((slab_flags_t __force)0x00000400U)
/* DEBUG: Poison objects */
-#define SLAB_POISON ((slab_flags_t __force)0x00000800UL)
+#define SLAB_POISON ((slab_flags_t __force)0x00000800U)
/* Align objs on cache lines */
-#define SLAB_HWCACHE_ALIGN ((slab_flags_t __force)0x00002000UL)
+#define SLAB_HWCACHE_ALIGN ((slab_flags_t __force)0x00002000U)
/* Use GFP_DMA memory */
-#define SLAB_CACHE_DMA ((slab_flags_t __force)0x00004000UL)
+#define SLAB_CACHE_DMA ((slab_flags_t __force)0x00004000U)
/* DEBUG: Store the last owner for bug hunting */
-#define SLAB_STORE_USER ((slab_flags_t __force)0x00010000UL)
+#define SLAB_STORE_USER ((slab_flags_t __force)0x00010000U)
/* Panic if kmem_cache_create() fails */
-#define SLAB_PANIC ((slab_flags_t __force)0x00040000UL)
+#define SLAB_PANIC ((slab_flags_t __force)0x00040000U)
/*
* SLAB_TYPESAFE_BY_RCU - **WARNING** READ THIS!
*
@@ -72,50 +72,50 @@
* Note that SLAB_TYPESAFE_BY_RCU was originally named SLAB_DESTROY_BY_RCU.
*/
/* Defer freeing slabs to RCU */
-#define SLAB_TYPESAFE_BY_RCU ((slab_flags_t __force)0x00080000UL)
+#define SLAB_TYPESAFE_BY_RCU ((slab_flags_t __force)0x00080000U)
/* Spread some memory over cpuset */
-#define SLAB_MEM_SPREAD ((slab_flags_t __force)0x00100000UL)
+#define SLAB_MEM_SPREAD ((slab_flags_t __force)0x00100000U)
/* Trace allocations and frees */
-#define SLAB_TRACE ((slab_flags_t __force)0x00200000UL)
+#define SLAB_TRACE ((slab_flags_t __force)0x00200000U)
/* Flag to prevent checks on free */
#ifdef CONFIG_DEBUG_OBJECTS
-# define SLAB_DEBUG_OBJECTS ((slab_flags_t __force)0x00400000UL)
+# define SLAB_DEBUG_OBJECTS ((slab_flags_t __force)0x00400000U)
#else
-# define SLAB_DEBUG_OBJECTS ((slab_flags_t __force)0x00000000UL)
+# define SLAB_DEBUG_OBJECTS 0
#endif
/* Avoid kmemleak tracing */
-#define SLAB_NOLEAKTRACE ((slab_flags_t __force)0x00800000UL)
+#define SLAB_NOLEAKTRACE ((slab_flags_t __force)0x00800000U)
/* Don't track use of uninitialized memory */
#ifdef CONFIG_KMEMCHECK
-# define SLAB_NOTRACK ((slab_flags_t __force)0x01000000UL)
+# define SLAB_NOTRACK ((slab_flags_t __force)0x01000000U)
#else
-# define SLAB_NOTRACK ((slab_flags_t __force)0x00000000UL)
+# define SLAB_NOTRACK 0
#endif
/* Fault injection mark */
#ifdef CONFIG_FAILSLAB
-# define SLAB_FAILSLAB ((slab_flags_t __force)0x02000000UL)
+# define SLAB_FAILSLAB ((slab_flags_t __force)0x02000000U)
#else
-# define SLAB_FAILSLAB ((slab_flags_t __force)0x00000000UL)
+# define SLAB_FAILSLAB 0
#endif
/* Account to memcg */
#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
-# define SLAB_ACCOUNT ((slab_flags_t __force)0x04000000UL)
+# define SLAB_ACCOUNT ((slab_flags_t __force)0x04000000U)
#else
-# define SLAB_ACCOUNT ((slab_flags_t __force)0x00000000UL)
+# define SLAB_ACCOUNT 0
#endif
#ifdef CONFIG_KASAN
-#define SLAB_KASAN ((slab_flags_t __force)0x08000000UL)
+#define SLAB_KASAN ((slab_flags_t __force)0x08000000U)
#else
-#define SLAB_KASAN ((slab_flags_t __force)0x00000000UL)
+#define SLAB_KASAN 0
#endif
/* The following flags affect the page allocator grouping pages by mobility */
/* Objects are reclaimable */
-#define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000UL)
+#define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000U)
#define SLAB_TEMPORARY SLAB_RECLAIM_ACCOUNT /* Objects are short-lived */
/*
* ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -155,7 +155,7 @@ typedef u32 dma_addr_t;
#endif
typedef unsigned __bitwise gfp_t;
-typedef unsigned long __bitwise slab_flags_t;
+typedef unsigned __bitwise slab_flags_t;
typedef unsigned __bitwise fmode_t;
#ifdef CONFIG_PHYS_ADDR_T_64BIT
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -251,8 +251,8 @@ static void kmem_cache_node_init(struct kmem_cache_node *parent)
MAKE_LIST((cachep), (&(ptr)->slabs_free), slabs_free, nodeid); \
} while (0)
-#define CFLGS_OBJFREELIST_SLAB ((slab_flags_t __force)0x40000000UL)
-#define CFLGS_OFF_SLAB ((slab_flags_t __force)0x80000000UL)
+#define CFLGS_OBJFREELIST_SLAB ((slab_flags_t __force)0x40000000U)
+#define CFLGS_OFF_SLAB ((slab_flags_t __force)0x80000000U)
#define OBJFREELIST_SLAB(x) ((x)->flags & CFLGS_OBJFREELIST_SLAB)
#define OFF_SLAB(x) ((x)->flags & CFLGS_OFF_SLAB)
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -193,9 +193,9 @@ static inline bool kmem_cache_has_cpu_partial(struct kmem_cache *s)
/* Internal SLUB flags */
/* Poison object */
-#define __OBJECT_POISON ((slab_flags_t __force)0x80000000UL)
+#define __OBJECT_POISON ((slab_flags_t __force)0x80000000U)
/* Use cmpxchg_double */
-#define __CMPXCHG_DOUBLE ((slab_flags_t __force)0x40000000UL)
+#define __CMPXCHG_DOUBLE ((slab_flags_t __force)0x40000000U)
/*
* Tracking user of a slab.
^ permalink raw reply
* Re: [PATCH v3 1/4] phylib: Add device reset GPIO support
From: Sergei Shtylyov @ 2017-10-21 10:03 UTC (permalink / raw)
To: Florian Fainelli, Geert Uytterhoeven, David S . Miller,
Andrew Lunn, Simon Horman, Magnus Damm
Cc: Rob Herring, Mark Rutland, Nicolas Ferre, netdev, devicetree,
linux-renesas-soc, linux-kernel
In-Reply-To: <2dacbfaf-ce5e-f113-9ae8-641441d99bbc@gmail.com>
Hello!
On 10/20/2017 9:11 PM, Florian Fainelli wrote:
>> From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> The PHY devices sometimes do have their reset signal (maybe even power
>> supply?) tied to some GPIO and sometimes it also does happen that a boot
>> loader does not leave it deasserted. So far this issue has been attacked
>> from (as I believe) a wrong angle: by teaching the MAC driver to manipulate
>> the GPIO in question; that solution, when applied to the device trees, led
>> to adding the PHY reset GPIO properties to the MAC device node, with one
>> exception: Cadence MACB driver which could handle the "reset-gpios" prop
>> in a PHY device subnode. I believe that the correct approach is to teach
>> the 'phylib' to get the MDIO device reset GPIO from the device tree node
>> corresponding to this device -- which this patch is doing...
>>
>> Note that I had to modify the AT803x PHY driver as it would stop working
>> otherwise -- it made use of the reset GPIO for its own purposes...
>>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Acked-by: Florian Fainelli <f.fainelli@gmail.com>
>
> This is a new patch as far as PHYLIB is concerned, so not quite accurate
No, it is not new. No changes in logic since v2.
> anymore. Thanks for picking that up, this looks good, with a few minor
> things here and there.
>> [geert: Propagate actual errors from fwnode_get_named_gpiod()]
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
[...]
>> diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
>> index 5f93e6add56394f2..15b4560aeb5de759 100644
>> --- a/drivers/net/phy/at803x.c
>> +++ b/drivers/net/phy/at803x.c
>
> You most certainly want to make the conversion of the at803x.c driver as
> a separate patch, there is no reason why it should be folded within the
No, it can't be a separate patch, otherwise I would have posted it
separately. We cannot request the same GPIO 2 times.
> patch teaching PHYLIB about PHY reset through GPIOs. More on that below.
>
[...]
>> @@ -254,22 +253,11 @@ static int at803x_probe(struct phy_device *phydev)
>> {
>> struct device *dev = &phydev->mdio.dev;
>> struct at803x_priv *priv;
>> - struct gpio_desc *gpiod_reset;
>>
>> priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> if (!priv)
>> return -ENOMEM;
>>
>> - if (phydev->drv->phy_id != ATH8030_PHY_ID)
>> - goto does_not_require_reset_workaround;
>
> We have lost that bit now, see below.
Hm... I'm still seeing this in net-next.
>> -
>> - gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW);
>> - if (IS_ERR(gpiod_reset))
>> - return PTR_ERR(gpiod_reset);
>> -
>> - priv->gpiod_reset = gpiod_reset;
>> -
>> -does_not_require_reset_workaround:
>> phydev->priv = priv;
>>
>> return 0;
>> @@ -343,14 +331,14 @@ static void at803x_link_change_notify(struct phy_device *phydev)
>> * cannot recover from by software.
>> */
>> if (phydev->state == PHY_NOLINK) {
>> - if (priv->gpiod_reset && !priv->phy_reset) {
>> + if (phydev->mdio.reset && !priv->phy_reset) {
>
> and phydev->drv->phy_id == ATH8030_PHY_ID, the workaround is not
> applicable to all PHY devices.
You are clearly looking at an outdated tree -- this check was removed,
This method is populated only for 8030 instead.
[...]
>> diff --git a/drivers/of/of_mdio.c b/drivers/of/of_mdio.c
>> index 98258583abb0b405..3d044119c032d176 100644
>> --- a/drivers/of/of_mdio.c
>> +++ b/drivers/of/of_mdio.c
[...]
>> @@ -55,10 +56,22 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
>> is_c45 = of_device_is_compatible(child,
>> "ethernet-phy-ieee802.3-c45");
>>
>> + /* Deassert the optional reset signal */
>> + gpiod = fwnode_get_named_gpiod(&child->fwnode, "reset-gpios", 0,
>> + GPIOD_OUT_LOW, "PHY reset");
>> + if (PTR_ERR(gpiod) == -ENOENT)
>> + gpiod = NULL;
>> + else if (IS_ERR(gpiod))
>> + return PTR_ERR(gpiod);
>> +
>> if (!is_c45 && !of_get_phy_id(child, &phy_id))
>> phy = phy_device_create(mdio, addr, phy_id, 0, NULL);
>> else
>> phy = get_phy_device(mdio, addr, is_c45);
>> +
>> + /* Assert the reset signal again */
>> + gpiod_set_value(gpiod, 1);
>
> You have a phy_device reference now, so why not call phy_device_reset()
> directly here?
Symmetry, perhaps? (There was a gpiod_set_value(gpiod, 0) call above it...
[...]
>> @@ -112,6 +127,14 @@ static int of_mdiobus_register_device(struct mii_bus *mdio,
>> of_node_get(child);
>> mdiodev->dev.of_node = child;
>>
>> + gpiod = fwnode_get_named_gpiod(&child->fwnode, "reset-gpios", 0,
>> + GPIOD_ASIS, "PHY reset");
>> + if (PTR_ERR(gpiod) == -ENOENT)
>> + gpiod = NULL;
>> + else if (IS_ERR(gpiod))
>> + return PTR_ERR(gpiod);
>> + mdiodev->reset = gpiod;
>
> There is some obvious and non desireable duplication of code between
> "pure" mdio_device and phy_device paths here, can you factor this such
> that there is a common function storing gpiod into a mdio_device's reset
> member in both cases?
>
> Also, there is some asymetry being exposed here, the GPIO descriptor is
> acquired from OF specific code, but is released in the generic MDIO
> device layer, can we find a way to make that symmetrical?
Finally move of_mdio.c into drivers/net/phy/? ;-)
MBR, Sergei
^ permalink raw reply
* Re: [PATCH v5 net-next 01/12] iptunnel: Add common functions to get a tunnel route
From: kbuild test robot @ 2017-10-21 10:01 UTC (permalink / raw)
To: Tom Herbert
Cc: kbuild-all, davem, pablo, laforge, aschultz, netdev, rohit,
Tom Herbert
In-Reply-To: <20171018201018.5692-2-tom@quantonium.net>
[-- Attachment #1: Type: text/plain, Size: 846 bytes --]
Hi Tom,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Tom-Herbert/gtp-Additional-feature-support-Part-I/20171021-121328
config: arm-iop33x_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=arm
All errors (new ones prefixed by >>):
net/ipv6/route.o: In function `__ip6_tnl_get_route':
>> route.c:(.text+0x1398): undefined reference to `dst_cache_set_ip6'
---
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: 15322 bytes --]
^ permalink raw reply
* [PATCH 1/2] slab, slub, slob: add slab_flags_t
From: Alexey Dobriyan @ 2017-10-21 10:02 UTC (permalink / raw)
To: akpm
Cc: linux-mm, cl, penberg, rientjes, iamjoonsoo.kim, ecryptfs,
linux-xfs, kasan-dev, netdev
Add sparse-checked slab_flags_t for struct kmem_cache::flags
(SLAB_POISON, etc).
SLAB is bloated temporarily by switching to "unsigned long",
but only temporarily.
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
fs/ecryptfs/main.c | 2 -
fs/xfs/kmem.h | 2 -
include/linux/kasan.h | 4 +--
include/linux/kmemleak.h | 8 +++---
include/linux/slab.h | 60 ++++++++++++++++++++++++++++-------------------
include/linux/slab_def.h | 2 -
include/linux/slub_def.h | 2 -
include/linux/types.h | 1
include/net/sock.h | 2 -
mm/kasan/kasan.c | 2 -
mm/slab.c | 23 ++++++++----------
mm/slab.h | 26 ++++++++++----------
mm/slab_common.c | 16 ++++++------
mm/slob.c | 2 -
mm/slub.c | 26 ++++++++++----------
15 files changed, 97 insertions(+), 81 deletions(-)
--- a/fs/ecryptfs/main.c
+++ b/fs/ecryptfs/main.c
@@ -660,7 +660,7 @@ static struct ecryptfs_cache_info {
struct kmem_cache **cache;
const char *name;
size_t size;
- unsigned long flags;
+ slab_flags_t flags;
void (*ctor)(void *obj);
} ecryptfs_cache_infos[] = {
{
--- a/fs/xfs/kmem.h
+++ b/fs/xfs/kmem.h
@@ -104,7 +104,7 @@ kmem_zone_init(int size, char *zone_name)
}
static inline kmem_zone_t *
-kmem_zone_init_flags(int size, char *zone_name, unsigned long flags,
+kmem_zone_init_flags(int size, char *zone_name, slab_flags_t flags,
void (*construct)(void *))
{
return kmem_cache_create(zone_name, size, 0, flags, construct);
--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -45,7 +45,7 @@ void kasan_alloc_pages(struct page *page, unsigned int order);
void kasan_free_pages(struct page *page, unsigned int order);
void kasan_cache_create(struct kmem_cache *cache, size_t *size,
- unsigned long *flags);
+ slab_flags_t *flags);
void kasan_cache_shrink(struct kmem_cache *cache);
void kasan_cache_shutdown(struct kmem_cache *cache);
@@ -94,7 +94,7 @@ static inline void kasan_free_pages(struct page *page, unsigned int order) {}
static inline void kasan_cache_create(struct kmem_cache *cache,
size_t *size,
- unsigned long *flags) {}
+ slab_flags_t *flags) {}
static inline void kasan_cache_shrink(struct kmem_cache *cache) {}
static inline void kasan_cache_shutdown(struct kmem_cache *cache) {}
--- a/include/linux/kmemleak.h
+++ b/include/linux/kmemleak.h
@@ -48,14 +48,14 @@ extern void kmemleak_not_leak_phys(phys_addr_t phys) __ref;
extern void kmemleak_ignore_phys(phys_addr_t phys) __ref;
static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
- int min_count, unsigned long flags,
+ int min_count, slab_flags_t flags,
gfp_t gfp)
{
if (!(flags & SLAB_NOLEAKTRACE))
kmemleak_alloc(ptr, size, min_count, gfp);
}
-static inline void kmemleak_free_recursive(const void *ptr, unsigned long flags)
+static inline void kmemleak_free_recursive(const void *ptr, slab_flags_t flags)
{
if (!(flags & SLAB_NOLEAKTRACE))
kmemleak_free(ptr);
@@ -76,7 +76,7 @@ static inline void kmemleak_alloc(const void *ptr, size_t size, int min_count,
{
}
static inline void kmemleak_alloc_recursive(const void *ptr, size_t size,
- int min_count, unsigned long flags,
+ int min_count, slab_flags_t flags,
gfp_t gfp)
{
}
@@ -94,7 +94,7 @@ static inline void kmemleak_free(const void *ptr)
static inline void kmemleak_free_part(const void *ptr, size_t size)
{
}
-static inline void kmemleak_free_recursive(const void *ptr, unsigned long flags)
+static inline void kmemleak_free_recursive(const void *ptr, slab_flags_t flags)
{
}
static inline void kmemleak_free_percpu(const void __percpu *ptr)
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -20,13 +20,20 @@
* Flags to pass to kmem_cache_create().
* The ones marked DEBUG are only valid if CONFIG_DEBUG_SLAB is set.
*/
-#define SLAB_CONSISTENCY_CHECKS 0x00000100UL /* DEBUG: Perform (expensive) checks on alloc/free */
-#define SLAB_RED_ZONE 0x00000400UL /* DEBUG: Red zone objs in a cache */
-#define SLAB_POISON 0x00000800UL /* DEBUG: Poison objects */
-#define SLAB_HWCACHE_ALIGN 0x00002000UL /* Align objs on cache lines */
-#define SLAB_CACHE_DMA 0x00004000UL /* Use GFP_DMA memory */
-#define SLAB_STORE_USER 0x00010000UL /* DEBUG: Store the last owner for bug hunting */
-#define SLAB_PANIC 0x00040000UL /* Panic if kmem_cache_create() fails */
+/* DEBUG: Perform (expensive) checks on alloc/free */
+#define SLAB_CONSISTENCY_CHECKS ((slab_flags_t __force)0x00000100UL)
+/* DEBUG: Red zone objs in a cache */
+#define SLAB_RED_ZONE ((slab_flags_t __force)0x00000400UL)
+/* DEBUG: Poison objects */
+#define SLAB_POISON ((slab_flags_t __force)0x00000800UL)
+/* Align objs on cache lines */
+#define SLAB_HWCACHE_ALIGN ((slab_flags_t __force)0x00002000UL)
+/* Use GFP_DMA memory */
+#define SLAB_CACHE_DMA ((slab_flags_t __force)0x00004000UL)
+/* DEBUG: Store the last owner for bug hunting */
+#define SLAB_STORE_USER ((slab_flags_t __force)0x00010000UL)
+/* Panic if kmem_cache_create() fails */
+#define SLAB_PANIC ((slab_flags_t __force)0x00040000UL)
/*
* SLAB_TYPESAFE_BY_RCU - **WARNING** READ THIS!
*
@@ -64,44 +71,51 @@
*
* Note that SLAB_TYPESAFE_BY_RCU was originally named SLAB_DESTROY_BY_RCU.
*/
-#define SLAB_TYPESAFE_BY_RCU 0x00080000UL /* Defer freeing slabs to RCU */
-#define SLAB_MEM_SPREAD 0x00100000UL /* Spread some memory over cpuset */
-#define SLAB_TRACE 0x00200000UL /* Trace allocations and frees */
+/* Defer freeing slabs to RCU */
+#define SLAB_TYPESAFE_BY_RCU ((slab_flags_t __force)0x00080000UL)
+/* Spread some memory over cpuset */
+#define SLAB_MEM_SPREAD ((slab_flags_t __force)0x00100000UL)
+/* Trace allocations and frees */
+#define SLAB_TRACE ((slab_flags_t __force)0x00200000UL)
/* Flag to prevent checks on free */
#ifdef CONFIG_DEBUG_OBJECTS
-# define SLAB_DEBUG_OBJECTS 0x00400000UL
+# define SLAB_DEBUG_OBJECTS ((slab_flags_t __force)0x00400000UL)
#else
-# define SLAB_DEBUG_OBJECTS 0x00000000UL
+# define SLAB_DEBUG_OBJECTS ((slab_flags_t __force)0x00000000UL)
#endif
-#define SLAB_NOLEAKTRACE 0x00800000UL /* Avoid kmemleak tracing */
+/* Avoid kmemleak tracing */
+#define SLAB_NOLEAKTRACE ((slab_flags_t __force)0x00800000UL)
/* Don't track use of uninitialized memory */
#ifdef CONFIG_KMEMCHECK
-# define SLAB_NOTRACK 0x01000000UL
+# define SLAB_NOTRACK ((slab_flags_t __force)0x01000000UL)
#else
-# define SLAB_NOTRACK 0x00000000UL
+# define SLAB_NOTRACK ((slab_flags_t __force)0x00000000UL)
#endif
+/* Fault injection mark */
#ifdef CONFIG_FAILSLAB
-# define SLAB_FAILSLAB 0x02000000UL /* Fault injection mark */
+# define SLAB_FAILSLAB ((slab_flags_t __force)0x02000000UL)
#else
-# define SLAB_FAILSLAB 0x00000000UL
+# define SLAB_FAILSLAB ((slab_flags_t __force)0x00000000UL)
#endif
+/* Account to memcg */
#if defined(CONFIG_MEMCG) && !defined(CONFIG_SLOB)
-# define SLAB_ACCOUNT 0x04000000UL /* Account to memcg */
+# define SLAB_ACCOUNT ((slab_flags_t __force)0x04000000UL)
#else
-# define SLAB_ACCOUNT 0x00000000UL
+# define SLAB_ACCOUNT ((slab_flags_t __force)0x00000000UL)
#endif
#ifdef CONFIG_KASAN
-#define SLAB_KASAN 0x08000000UL
+#define SLAB_KASAN ((slab_flags_t __force)0x08000000UL)
#else
-#define SLAB_KASAN 0x00000000UL
+#define SLAB_KASAN ((slab_flags_t __force)0x00000000UL)
#endif
/* The following flags affect the page allocator grouping pages by mobility */
-#define SLAB_RECLAIM_ACCOUNT 0x00020000UL /* Objects are reclaimable */
+/* Objects are reclaimable */
+#define SLAB_RECLAIM_ACCOUNT ((slab_flags_t __force)0x00020000UL)
#define SLAB_TEMPORARY SLAB_RECLAIM_ACCOUNT /* Objects are short-lived */
/*
* ZERO_SIZE_PTR will be returned for zero sized kmalloc requests.
@@ -127,7 +141,7 @@ void __init kmem_cache_init(void);
bool slab_is_available(void);
struct kmem_cache *kmem_cache_create(const char *, size_t, size_t,
- unsigned long,
+ slab_flags_t,
void (*)(void *));
void kmem_cache_destroy(struct kmem_cache *);
int kmem_cache_shrink(struct kmem_cache *);
--- a/include/linux/slab_def.h
+++ b/include/linux/slab_def.h
@@ -19,7 +19,7 @@ struct kmem_cache {
struct reciprocal_value reciprocal_buffer_size;
/* 2) touched by every alloc & free from the backend */
- unsigned int flags; /* constant flags */
+ slab_flags_t flags; /* constant flags */
unsigned int num; /* # of objs per slab */
/* 3) cache_grow/shrink */
--- a/include/linux/slub_def.h
+++ b/include/linux/slub_def.h
@@ -81,7 +81,7 @@ struct kmem_cache_order_objects {
struct kmem_cache {
struct kmem_cache_cpu __percpu *cpu_slab;
/* Used for retriving partial slabs etc */
- unsigned long flags;
+ slab_flags_t flags;
unsigned long min_partial;
int size; /* The size of an object including meta data */
int object_size; /* The size of an object without meta data */
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -155,6 +155,7 @@ typedef u32 dma_addr_t;
#endif
typedef unsigned __bitwise gfp_t;
+typedef unsigned long __bitwise slab_flags_t;
typedef unsigned __bitwise fmode_t;
#ifdef CONFIG_PHYS_ADDR_T_64BIT
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1105,7 +1105,7 @@ struct proto {
struct kmem_cache *slab;
unsigned int obj_size;
- int slab_flags;
+ slab_flags_t slab_flags;
struct percpu_counter *orphan_count;
--- a/mm/kasan/kasan.c
+++ b/mm/kasan/kasan.c
@@ -337,7 +337,7 @@ static size_t optimal_redzone(size_t object_size)
}
void kasan_cache_create(struct kmem_cache *cache, size_t *size,
- unsigned long *flags)
+ slab_flags_t *flags)
{
int redzone_adjust;
int orig_size = *size;
--- a/mm/slab.c
+++ b/mm/slab.c
@@ -251,8 +251,8 @@ static void kmem_cache_node_init(struct kmem_cache_node *parent)
MAKE_LIST((cachep), (&(ptr)->slabs_free), slabs_free, nodeid); \
} while (0)
-#define CFLGS_OBJFREELIST_SLAB (0x40000000UL)
-#define CFLGS_OFF_SLAB (0x80000000UL)
+#define CFLGS_OBJFREELIST_SLAB ((slab_flags_t __force)0x40000000UL)
+#define CFLGS_OFF_SLAB ((slab_flags_t __force)0x80000000UL)
#define OBJFREELIST_SLAB(x) ((x)->flags & CFLGS_OBJFREELIST_SLAB)
#define OFF_SLAB(x) ((x)->flags & CFLGS_OFF_SLAB)
@@ -440,7 +440,7 @@ static inline struct array_cache *cpu_cache_get(struct kmem_cache *cachep)
* Calculate the number of objects and left-over bytes for a given buffer size.
*/
static unsigned int cache_estimate(unsigned long gfporder, size_t buffer_size,
- unsigned long flags, size_t *left_over)
+ slab_flags_t flags, size_t *left_over)
{
unsigned int num;
size_t slab_size = PAGE_SIZE << gfporder;
@@ -1760,7 +1760,7 @@ static void slabs_destroy(struct kmem_cache *cachep, struct list_head *list)
* towards high-order requests, this should be changed.
*/
static size_t calculate_slab_order(struct kmem_cache *cachep,
- size_t size, unsigned long flags)
+ size_t size, slab_flags_t flags)
{
size_t left_over = 0;
int gfporder;
@@ -1887,8 +1887,8 @@ static int __ref setup_cpu_cache(struct kmem_cache *cachep, gfp_t gfp)
return 0;
}
-unsigned long kmem_cache_flags(unsigned long object_size,
- unsigned long flags, const char *name,
+slab_flags_t kmem_cache_flags(unsigned long object_size,
+ slab_flags_t flags, const char *name,
void (*ctor)(void *))
{
return flags;
@@ -1896,7 +1896,7 @@ unsigned long kmem_cache_flags(unsigned long object_size,
struct kmem_cache *
__kmem_cache_alias(const char *name, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *))
+ slab_flags_t flags, void (*ctor)(void *))
{
struct kmem_cache *cachep;
@@ -1914,7 +1914,7 @@ __kmem_cache_alias(const char *name, size_t size, size_t align,
}
static bool set_objfreelist_slab_cache(struct kmem_cache *cachep,
- size_t size, unsigned long flags)
+ size_t size, slab_flags_t flags)
{
size_t left;
@@ -1937,7 +1937,7 @@ static bool set_objfreelist_slab_cache(struct kmem_cache *cachep,
}
static bool set_off_slab_cache(struct kmem_cache *cachep,
- size_t size, unsigned long flags)
+ size_t size, slab_flags_t flags)
{
size_t left;
@@ -1971,7 +1971,7 @@ static bool set_off_slab_cache(struct kmem_cache *cachep,
}
static bool set_on_slab_cache(struct kmem_cache *cachep,
- size_t size, unsigned long flags)
+ size_t size, slab_flags_t flags)
{
size_t left;
@@ -2007,8 +2007,7 @@ static bool set_on_slab_cache(struct kmem_cache *cachep,
* cacheline. This can be beneficial if you're counting cycles as closely
* as davem.
*/
-int
-__kmem_cache_create (struct kmem_cache *cachep, unsigned long flags)
+int __kmem_cache_create(struct kmem_cache *cachep, slab_flags_t flags)
{
size_t ralign = BYTES_PER_WORD;
gfp_t gfp;
--- a/mm/slab.h
+++ b/mm/slab.h
@@ -20,7 +20,7 @@ struct kmem_cache {
unsigned int object_size;/* The original size of the object */
unsigned int size; /* The aligned/padded/added on size */
unsigned int align; /* Alignment as calculated */
- unsigned long flags; /* Active flags on the slab */
+ slab_flags_t flags; /* Active flags on the slab */
const char *name; /* Slab name for sysfs */
int refcount; /* Use counter */
void (*ctor)(void *); /* Called on object slot creation */
@@ -78,13 +78,13 @@ extern const struct kmalloc_info_struct {
unsigned long size;
} kmalloc_info[];
-unsigned long calculate_alignment(unsigned long flags,
+unsigned long calculate_alignment(slab_flags_t flags,
unsigned long align, unsigned long size);
#ifndef CONFIG_SLOB
/* Kmalloc array related functions */
void setup_kmalloc_cache_index_table(void);
-void create_kmalloc_caches(unsigned long);
+void create_kmalloc_caches(slab_flags_t);
/* Find the kmalloc slab corresponding for a certain size */
struct kmem_cache *kmalloc_slab(size_t, gfp_t);
@@ -92,32 +92,32 @@ struct kmem_cache *kmalloc_slab(size_t, gfp_t);
/* Functions provided by the slab allocators */
-extern int __kmem_cache_create(struct kmem_cache *, unsigned long flags);
+int __kmem_cache_create(struct kmem_cache *, slab_flags_t flags);
extern struct kmem_cache *create_kmalloc_cache(const char *name, size_t size,
- unsigned long flags);
+ slab_flags_t flags);
extern void create_boot_cache(struct kmem_cache *, const char *name,
- size_t size, unsigned long flags);
+ size_t size, slab_flags_t flags);
int slab_unmergeable(struct kmem_cache *s);
struct kmem_cache *find_mergeable(size_t size, size_t align,
- unsigned long flags, const char *name, void (*ctor)(void *));
+ slab_flags_t flags, const char *name, void (*ctor)(void *));
#ifndef CONFIG_SLOB
struct kmem_cache *
__kmem_cache_alias(const char *name, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *));
+ slab_flags_t flags, void (*ctor)(void *));
-unsigned long kmem_cache_flags(unsigned long object_size,
- unsigned long flags, const char *name,
+slab_flags_t kmem_cache_flags(unsigned long object_size,
+ slab_flags_t flags, const char *name,
void (*ctor)(void *));
#else
static inline struct kmem_cache *
__kmem_cache_alias(const char *name, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *))
+ slab_flags_t flags, void (*ctor)(void *))
{ return NULL; }
-static inline unsigned long kmem_cache_flags(unsigned long object_size,
- unsigned long flags, const char *name,
+static inline slab_flags_t kmem_cache_flags(unsigned long object_size,
+ slab_flags_t flags, const char *name,
void (*ctor)(void *))
{
return flags;
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -290,7 +290,7 @@ int slab_unmergeable(struct kmem_cache *s)
}
struct kmem_cache *find_mergeable(size_t size, size_t align,
- unsigned long flags, const char *name, void (*ctor)(void *))
+ slab_flags_t flags, const char *name, void (*ctor)(void *))
{
struct kmem_cache *s;
@@ -340,7 +340,7 @@ struct kmem_cache *find_mergeable(size_t size, size_t align,
* Figure out what the alignment of the objects will be given a set of
* flags, a user specified alignment and the size of the objects.
*/
-unsigned long calculate_alignment(unsigned long flags,
+unsigned long calculate_alignment(slab_flags_t flags,
unsigned long align, unsigned long size)
{
/*
@@ -365,7 +365,7 @@ unsigned long calculate_alignment(unsigned long flags,
static struct kmem_cache *create_cache(const char *name,
size_t object_size, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *),
+ slab_flags_t flags, void (*ctor)(void *),
struct mem_cgroup *memcg, struct kmem_cache *root_cache)
{
struct kmem_cache *s;
@@ -430,7 +430,7 @@ static struct kmem_cache *create_cache(const char *name,
*/
struct kmem_cache *
kmem_cache_create(const char *name, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *))
+ slab_flags_t flags, void (*ctor)(void *))
{
struct kmem_cache *s = NULL;
const char *cache_name;
@@ -878,7 +878,7 @@ bool slab_is_available(void)
#ifndef CONFIG_SLOB
/* Create a cache during boot when no slab services are available yet */
void __init create_boot_cache(struct kmem_cache *s, const char *name, size_t size,
- unsigned long flags)
+ slab_flags_t flags)
{
int err;
@@ -898,7 +898,7 @@ void __init create_boot_cache(struct kmem_cache *s, const char *name, size_t siz
}
struct kmem_cache *__init create_kmalloc_cache(const char *name, size_t size,
- unsigned long flags)
+ slab_flags_t flags)
{
struct kmem_cache *s = kmem_cache_zalloc(kmem_cache, GFP_NOWAIT);
@@ -1056,7 +1056,7 @@ void __init setup_kmalloc_cache_index_table(void)
}
}
-static void __init new_kmalloc_cache(int idx, unsigned long flags)
+static void __init new_kmalloc_cache(int idx, slab_flags_t flags)
{
kmalloc_caches[idx] = create_kmalloc_cache(kmalloc_info[idx].name,
kmalloc_info[idx].size, flags);
@@ -1067,7 +1067,7 @@ static void __init new_kmalloc_cache(int idx, unsigned long flags)
* may already have been created because they were needed to
* enable allocations for slab creation.
*/
-void __init create_kmalloc_caches(unsigned long flags)
+void __init create_kmalloc_caches(slab_flags_t flags)
{
int i;
--- a/mm/slob.c
+++ b/mm/slob.c
@@ -523,7 +523,7 @@ size_t ksize(const void *block)
}
EXPORT_SYMBOL(ksize);
-int __kmem_cache_create(struct kmem_cache *c, unsigned long flags)
+int __kmem_cache_create(struct kmem_cache *c, slab_flags_t flags)
{
if (flags & SLAB_TYPESAFE_BY_RCU) {
/* leave room for rcu footer at the end of object */
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -192,8 +192,10 @@ static inline bool kmem_cache_has_cpu_partial(struct kmem_cache *s)
#define MAX_OBJS_PER_PAGE 32767 /* since page.objects is u15 */
/* Internal SLUB flags */
-#define __OBJECT_POISON 0x80000000UL /* Poison object */
-#define __CMPXCHG_DOUBLE 0x40000000UL /* Use cmpxchg_double */
+/* Poison object */
+#define __OBJECT_POISON ((slab_flags_t __force)0x80000000UL)
+/* Use cmpxchg_double */
+#define __CMPXCHG_DOUBLE ((slab_flags_t __force)0x40000000UL)
/*
* Tracking user of a slab.
@@ -484,9 +486,9 @@ static inline void *restore_red_left(struct kmem_cache *s, void *p)
* Debug settings:
*/
#if defined(CONFIG_SLUB_DEBUG_ON)
-static int slub_debug = DEBUG_DEFAULT_FLAGS;
+static slab_flags_t slub_debug = DEBUG_DEFAULT_FLAGS;
#else
-static int slub_debug;
+static slab_flags_t slub_debug;
#endif
static char *slub_debug_slabs;
@@ -1288,8 +1290,8 @@ static int __init setup_slub_debug(char *str)
__setup("slub_debug", setup_slub_debug);
-unsigned long kmem_cache_flags(unsigned long object_size,
- unsigned long flags, const char *name,
+slab_flags_t kmem_cache_flags(unsigned long object_size,
+ slab_flags_t flags, const char *name,
void (*ctor)(void *))
{
/*
@@ -1321,8 +1323,8 @@ static inline void add_full(struct kmem_cache *s, struct kmem_cache_node *n,
struct page *page) {}
static inline void remove_full(struct kmem_cache *s, struct kmem_cache_node *n,
struct page *page) {}
-unsigned long kmem_cache_flags(unsigned long object_size,
- unsigned long flags, const char *name,
+slab_flags_t kmem_cache_flags(unsigned long object_size,
+ slab_flags_t flags, const char *name,
void (*ctor)(void *))
{
return flags;
@@ -3476,7 +3478,7 @@ static void set_cpu_partial(struct kmem_cache *s)
*/
static int calculate_sizes(struct kmem_cache *s, int forced_order)
{
- unsigned long flags = s->flags;
+ slab_flags_t flags = s->flags;
size_t size = s->object_size;
int order;
@@ -3592,7 +3594,7 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
return !!oo_objects(s->oo);
}
-static int kmem_cache_open(struct kmem_cache *s, unsigned long flags)
+static int kmem_cache_open(struct kmem_cache *s, slab_flags_t flags)
{
s->flags = kmem_cache_flags(s->size, flags, s->name, s->ctor);
s->reserved = 0;
@@ -4244,7 +4246,7 @@ void __init kmem_cache_init_late(void)
struct kmem_cache *
__kmem_cache_alias(const char *name, size_t size, size_t align,
- unsigned long flags, void (*ctor)(void *))
+ slab_flags_t flags, void (*ctor)(void *))
{
struct kmem_cache *s, *c;
@@ -4274,7 +4276,7 @@ __kmem_cache_alias(const char *name, size_t size, size_t align,
return s;
}
-int __kmem_cache_create(struct kmem_cache *s, unsigned long flags)
+int __kmem_cache_create(struct kmem_cache *s, slab_flags_t flags)
{
int err;
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: Xin Long @ 2017-10-21 9:45 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, David Miller, network dev, Marcelo Ricardo Leitner,
Sabrina Dubroca
In-Reply-To: <CADvbK_cKxwVxVbJU1uTVN1Goz_Wju1q71a0qUDWXY-zLCYNhhw@mail.gmail.com>
On Sat, Oct 21, 2017 at 4:45 PM, Xin Long <lucien.xin@gmail.com> wrote:
> On Sat, Oct 21, 2017 at 3:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> On Sat, 2017-10-21 at 14:51 +0800, Xin Long wrote:
>>> On Sat, Oct 21, 2017 at 2:18 PM, Eric Dumazet <edumazet@google.com> wrote:
>>> > On Fri, Oct 20, 2017 at 11:06 PM, Xin Long <lucien.xin@gmail.com> wrote:
>>> >>
>>> >>
>>> >> On Sat, Oct 21, 2017 at 9:27 AM, David Miller <davem@davemloft.net> wrote:
>>> >>>
>>> >>> From: Xin Long <lucien.xin@gmail.com>
>>> >>> Date: Thu, 19 Oct 2017 15:32:23 +0800
>>> >>>
>>> >>> > This patch is to void the potential security issue that the family
>>> >>> > or protocol modules are autoloaded when requesting _diag module by
>>> >>> > not requesting _diag module if the family or protocol is not added
>>> >>> > or registered in sock_diag and inet_diag.
>>> >>> >
>>> >>> > As the repost of the patch '[PATCH net] sock_diag: request _diag
>>> >>> > module only when the family or proto has been registered', this
>>> >>> > patchset fixes the compiling errors when INET is not set, and
>>> >>> > also split into two patches to make it clear to review.
>>> >>>
>>> >>> This makes no sense to me.
>>> >>>
>>> >>> Any user can just open a socket() in the appropriate protocol
>>> >>> family to cause the module to be loaded.
>>> >>>
>>> >>> If someone wants modules to not be loaded, block them using
>>> >>> traditional module loading infrastructure mechanisms. Or
>>> >>> don't load the module at all.
>>> >>>
>>> >>> Sorry I am not applying this.
>>> >>
>>> >> Hi David,
>>> >>
>>> >> I'm still thinking it's not good after 'ss', sctp, dccp,
>>> >> af_packet ... are just loaded, in which case, no one actually
>>> >> open any socket with these family or proto.
>>> >>
>>> >> I talked with Marcelo before, one scenario as he said:
>>> >>
>>> >> Imagine a customer generates a sosreport on their system, and
>>> >> with that, it loads sctp module. From then on, if their firewall
>>> >> doesn't block incoming packets for sctp, they may be prone to some
>>> >> remotely triggerable issue on sctp code, without even actually using
>>> >> sctp.
>>> >
>>> > For that reason, we have disabled autoloading of SCTP.
>>> > ( removing the
>>> > MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132");
>>> > MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132");
>>> > )
>>> > root must modprobe the module before it is accessible.
>>> >
>>> > However inet_diag is a way to have the module loaded anyway.
>>> >
>>> > This is why I like your patch Xin.
>>> >
>>> > David is only saying that your patch alone is not enough to prevent a
>>> > user to use socket() to autoload SCTP.
>>> Using socket() to autoload SCTP should be fine, cause users would
>>> use SCTP, no ?
>>>
>>> "ss" doesn't mean users intend to use SCTP, "ss" may make users
>>> not aware that SCTP module would be loaded, unlike socket(SCTP).
>>
>> Your changelog mentions a security issue.
>>
>> How have you prevented user using socket() to bypass your 'security'
>> feature ?
> I think the hook in __sock_create():
>
> err = security_socket_create(family, type, protocol, kern);
> if (err)
> return err;
>
> could work for this, no ?
Sorry, Eric, this may not be an inappropriate example, after all
security_socket_create is not supposed to do these things.
thanks for your review.
Let's just see if David could accept the patches if I will
remove the "security claim" from changelog, considering
it as an improvement of sock diag.
David ?
>
> like, users add security rules to not allow to create SCTP socket
> (namely, not allow sctp module to be loaded)
>
> But, 'ss' could bypass it to load SCTP module.
>
> In this way, can this patch be considered a security issue ?
>
>>
>> If you have not yet, this security claim is simply false.
^ permalink raw reply
* Re: [PATCH net-next v2 1/6] devlink: Add permanent config parameter get/set operations
From: Jiri Pirko @ 2017-10-21 9:24 UTC (permalink / raw)
To: Steve Lin
Cc: Linux Netdev List, Jiri Pirko, David S . Miller, Michael Chan,
John Linville, Andy Gospodarek
In-Reply-To: <CA+Jmh7GptV5EZYWpQzHnT8Q1KDSZ-juQCfOr=VKFt4jyMOqFYw@mail.gmail.com>
Fri, Oct 20, 2017 at 05:13:39PM CEST, steven.lin1@broadcom.com wrote:
>On Fri, Oct 20, 2017 at 10:39 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Thu, Oct 19, 2017 at 09:17:05PM CEST, steven.lin1@broadcom.com wrote:
>>>Add support for permanent config parameter get/set commands. Used
>>>for parameters held in NVRAM, persistent device configuration.
>>>
>>>Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>>>Acked-by: Andy Gospodarek <gospo@broadcom.com>
>>>---
>>> include/net/devlink.h | 3 +
>>> include/uapi/linux/devlink.h | 11 ++
>>> net/core/devlink.c | 234 +++++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 248 insertions(+)
>>>
>>>diff --git a/include/net/devlink.h b/include/net/devlink.h
>>>index b9654e1..bd64623 100644
>>>--- a/include/net/devlink.h
>>>+++ b/include/net/devlink.h
>>>@@ -270,6 +270,9 @@ struct devlink_ops {
>>> int (*eswitch_inline_mode_set)(struct devlink *devlink, u8 inline_mode);
>>> int (*eswitch_encap_mode_get)(struct devlink *devlink, u8 *p_encap_mode);
>>> int (*eswitch_encap_mode_set)(struct devlink *devlink, u8 encap_mode);
>>>+ int (*perm_config_get)(struct devlink *devlink, u32 param, u32 *value);
>>>+ int (*perm_config_set)(struct devlink *devlink, u32 param, u32 value,
>>
>> Please use enum instead of "u32 param". Also, what would happen if the
>> value is >u32, like string for example? I believe we need to take it into
>> the consideration for the UAPI sake.
>>
>>
>
>Using enum instead of u32 param: ok, will do in v3, thanks.
>
>Value > u32: In the RFC and v1 versions of the patch, each parameter
>was its own attribute, so could have its own type (u32, string,
>whatever). In v2, trying to move to nested parameters w/ parameter
>being an enum, as requested, it seems to mean that the parameter value
>now must be defined as a specific type, so I went with u32.
Why? I have to be missing something. In the nest all is same as outside
of the nest.
Also, please see team_nl_cmd_options_set() where something similar is
done, for multiple option types.
>
>If we need to support strings or other types > u32, then the
>perm_config_value attribute will not be a fixed type, so can't be
>policy checked. Or, I could go back to non-nested as in RFC/v1 case
>and have each parameter with its own type.
>
>> [...]
>>
>>
>>>+
>>>+static int devlink_nl_single_param_set(struct sk_buff *msg,
>>>+ struct devlink *devlink,
>>>+ u32 param, u32 value)
>>>+{
>>>+ u32 orig_value;
>>>+ u8 need_restart;
>>>+ int err;
>>>+ const struct devlink_ops *ops = devlink->ops;
>>>+ struct nlattr *cfgparam_attr;
>>
>> Reverse christmas tree please (this applies to all functions)
>
>Will do in v3, thanks.
>
>>
>>
>>>+
>>>+ /* First get current value of parameter */
>>>+ err = ops->perm_config_get(devlink, param, &orig_value);
>>
>> I'm missing why this is needed.
>>
>>
>>>+ if (err)
>>>+ return err;
>>>+
>>>+ /* Now set parameter */
>>>+ err = ops->perm_config_set(devlink, param, value, &need_restart);
>>>+ if (err)
>>>+ return err;
>>>+
>>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIG);
>>>+ /* Update restart reqd - if any param needs restart, should be set */
>>>+ if (need_restart)
>>>+ err = nla_put_u8(msg,
>>>+ DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED, 1);
>>>+
>>>+ /* Since set was successful, write attr back to msg with orig val */
>>>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_PARAMETER, param);
>>>+ err = nla_put_u32(msg, DEVLINK_ATTR_PERM_CONFIG_VALUE, orig_value);
>>
>> Why to write it back?
>
>In a response to the "RFC" version of this patch, you wrote: "Also,
>we need to expose to the user the original value (currently being
>used) and the new one (to be used after driver re-instatiation)".
>
>I understood that to mean that we need to return the current/original
>value of the parameter (and the user knows the new value, since they
>are setting it).
>
>If I mis-interpreted that comment, then I'm happy to remove returning
>the original value to the user; it wasn't in there originally.
>
>>
>>
>>>+
>>>+ nla_nest_end(msg, cfgparam_attr);
>>>+
>>>+ return 0;
>>>+}
>>>+
>>>+static int devlink_nl_cmd_perm_config_set_doit(struct sk_buff *skb,
>>>+ struct genl_info *info)
>>>+{
>>>+ struct devlink *devlink = info->user_ptr[0];
>>>+ struct sk_buff *msg;
>>>+ void *hdr;
>>>+ struct nlattr *attr;
>>>+ int rem;
>>>+ int err;
>>>+ u8 restart_reqd = 0;
>>>+ struct nlattr *cfgparam_attr;
>>>+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1];
>>>+ u32 param;
>>>+ u32 value;
>>>+
>>>+ if (!devlink->ops || !devlink->ops->perm_config_get ||
>>>+ !devlink->ops->perm_config_set)
>>>+ return -EOPNOTSUPP;
>>>+
>>>+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>>>+ if (!msg)
>>>+ return -ENOMEM;
>>>+
>>>+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq,
>>>+ &devlink_nl_family, 0, DEVLINK_CMD_PERM_CONFIG_SET);
>>>+ if (!hdr) {
>>>+ err = -EMSGSIZE;
>>>+ goto nla_msg_failure;
>>>+ }
>>>+
>>>+ err = devlink_nl_put_handle(msg, devlink);
>>>+ if (err)
>>>+ goto nla_put_failure;
>>>+
>>>+ cfgparam_attr = nla_nest_start(msg, DEVLINK_ATTR_PERM_CONFIGS);
>>>+
>>>+ nla_for_each_nested(attr, info->attrs[DEVLINK_ATTR_PERM_CONFIGS], rem) {
>>>+ err = nla_parse_nested(tb, DEVLINK_ATTR_MAX, attr,
>>>+ devlink_nl_policy, NULL);
>>>+ if (err)
>>>+ goto nla_nest_failure;
>>>+
>>>+ if (!tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER] ||
>>>+ !tb[DEVLINK_ATTR_PERM_CONFIG_VALUE])
>>>+ continue;
>>>+
>>>+ param = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_PARAMETER]);
>>
>> You should check it the "param" value is withing the enum boundary.
>
>Will do in v3, I'll add a DEVLINK_PERM_CONFIG_MAX enum and check against that.
>
>>
>>
>>>+ value = nla_get_u32(tb[DEVLINK_ATTR_PERM_CONFIG_VALUE]);
>>>+ err = devlink_nl_single_param_set(msg, devlink, param,
>>>+ value);
>>>+ if (err)
>>>+ goto nla_nest_failure;
>>
>> Wouldn't it make sense to rollback to old values if any of the config
>> parameters set would fail?
>
>Hmmmm.... We could do a rollback on failure of any of the set cmds,
>but I feel like that's more useful when the user doesn't know which
>sets failed. (i.e. if the response was just a boolean ok/not-ok
>response).
>
>If the user knows which sets worked and which didn't, then maybe no
>rollback is necessary. So perhaps I change the code so that rather
>than falling out on error, it continues to try all the set cmds, but
>just returns the parameters which were successful.
>
>Is there a convention for this? I'm fine either way.
>
>>
>>
>>>+ }
>>>+
>>>+ nla_nest_end(msg, cfgparam_attr);
>>>+
>>>+ if (restart_reqd) {
>>>+ err = nla_put_u8(msg, DEVLINK_ATTR_PERM_CONFIG_RESTART_REQUIRED,
>>>+ restart_reqd);
>>>+ if (err)
>>>+ goto nla_put_failure;
>>>+ }
>>>+
>>>+ genlmsg_end(msg, hdr);
>>>+ return genlmsg_reply(msg, info);
>>>+
>>>+nla_nest_failure:
>>>+ nla_nest_cancel(msg, cfgparam_attr);
>>>+nla_put_failure:
>>>+ genlmsg_cancel(msg, hdr);
>>>+nla_msg_failure:
>>>+ return err;
>>>+}
>>>+
>>> int devlink_dpipe_match_put(struct sk_buff *skb,
>>> struct devlink_dpipe_match *match)
>>> {
>>>@@ -2291,6 +2509,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
>>> [DEVLINK_ATTR_ESWITCH_ENCAP_MODE] = { .type = NLA_U8 },
>>> [DEVLINK_ATTR_DPIPE_TABLE_NAME] = { .type = NLA_NUL_STRING },
>>> [DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED] = { .type = NLA_U8 },
>>>+ [DEVLINK_ATTR_PERM_CONFIG_PARAMETER] = { .type = NLA_U32 },
>>>+ [DEVLINK_ATTR_PERM_CONFIG_VALUE] = { .type = NLA_U32 },
>>> };
>>>
>>> static const struct genl_ops devlink_nl_ops[] = {
>>>@@ -2451,6 +2671,20 @@ static const struct genl_ops devlink_nl_ops[] = {
>>> .flags = GENL_ADMIN_PERM,
>>> .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>>> },
>>>+ {
>>>+ .cmd = DEVLINK_CMD_PERM_CONFIG_GET,
>>>+ .doit = devlink_nl_cmd_perm_config_get_doit,
>>>+ .policy = devlink_nl_policy,
>>>+ .flags = GENL_ADMIN_PERM,
>>>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>>>+ },
>>>+ {
>>>+ .cmd = DEVLINK_CMD_PERM_CONFIG_SET,
>>>+ .doit = devlink_nl_cmd_perm_config_set_doit,
>>>+ .policy = devlink_nl_policy,
>>>+ .flags = GENL_ADMIN_PERM,
>>>+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
>>>+ },
>>> };
>>>
>>> static struct genl_family devlink_nl_family __ro_after_init = {
>>>--
>>>2.7.4
>>>
^ permalink raw reply
* Soliciting
From: Kim Sharma @ 2017-10-21 5:12 UTC (permalink / raw)
To: Recipients
Present Blue sky Studio, is Soliciting for the Right to use Your Photo/Face and Personality as One of the Semi -Major Role/ Character in our Upcoming ANIMATED Stereoscope 3D Movie-The Story of Anubis
All Reply to: blueskyentstudios@usa.com
^ permalink raw reply
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: Xin Long @ 2017-10-21 8:45 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric Dumazet, David Miller, network dev, Marcelo Ricardo Leitner,
Sabrina Dubroca
In-Reply-To: <1508571955.30291.21.camel@edumazet-glaptop3.roam.corp.google.com>
On Sat, Oct 21, 2017 at 3:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2017-10-21 at 14:51 +0800, Xin Long wrote:
>> On Sat, Oct 21, 2017 at 2:18 PM, Eric Dumazet <edumazet@google.com> wrote:
>> > On Fri, Oct 20, 2017 at 11:06 PM, Xin Long <lucien.xin@gmail.com> wrote:
>> >>
>> >>
>> >> On Sat, Oct 21, 2017 at 9:27 AM, David Miller <davem@davemloft.net> wrote:
>> >>>
>> >>> From: Xin Long <lucien.xin@gmail.com>
>> >>> Date: Thu, 19 Oct 2017 15:32:23 +0800
>> >>>
>> >>> > This patch is to void the potential security issue that the family
>> >>> > or protocol modules are autoloaded when requesting _diag module by
>> >>> > not requesting _diag module if the family or protocol is not added
>> >>> > or registered in sock_diag and inet_diag.
>> >>> >
>> >>> > As the repost of the patch '[PATCH net] sock_diag: request _diag
>> >>> > module only when the family or proto has been registered', this
>> >>> > patchset fixes the compiling errors when INET is not set, and
>> >>> > also split into two patches to make it clear to review.
>> >>>
>> >>> This makes no sense to me.
>> >>>
>> >>> Any user can just open a socket() in the appropriate protocol
>> >>> family to cause the module to be loaded.
>> >>>
>> >>> If someone wants modules to not be loaded, block them using
>> >>> traditional module loading infrastructure mechanisms. Or
>> >>> don't load the module at all.
>> >>>
>> >>> Sorry I am not applying this.
>> >>
>> >> Hi David,
>> >>
>> >> I'm still thinking it's not good after 'ss', sctp, dccp,
>> >> af_packet ... are just loaded, in which case, no one actually
>> >> open any socket with these family or proto.
>> >>
>> >> I talked with Marcelo before, one scenario as he said:
>> >>
>> >> Imagine a customer generates a sosreport on their system, and
>> >> with that, it loads sctp module. From then on, if their firewall
>> >> doesn't block incoming packets for sctp, they may be prone to some
>> >> remotely triggerable issue on sctp code, without even actually using
>> >> sctp.
>> >
>> > For that reason, we have disabled autoloading of SCTP.
>> > ( removing the
>> > MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132");
>> > MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132");
>> > )
>> > root must modprobe the module before it is accessible.
>> >
>> > However inet_diag is a way to have the module loaded anyway.
>> >
>> > This is why I like your patch Xin.
>> >
>> > David is only saying that your patch alone is not enough to prevent a
>> > user to use socket() to autoload SCTP.
>> Using socket() to autoload SCTP should be fine, cause users would
>> use SCTP, no ?
>>
>> "ss" doesn't mean users intend to use SCTP, "ss" may make users
>> not aware that SCTP module would be loaded, unlike socket(SCTP).
>
> Your changelog mentions a security issue.
>
> How have you prevented user using socket() to bypass your 'security'
> feature ?
I think the hook in __sock_create():
err = security_socket_create(family, type, protocol, kern);
if (err)
return err;
could work for this, no ?
like, users add security rules to not allow to create SCTP socket
(namely, not allow sctp module to be loaded)
But, 'ss' could bypass it to load SCTP module.
In this way, can this patch be considered a security issue ?
>
> If you have not yet, this security claim is simply false.
^ permalink raw reply
* Re: [PATCH net-next] selftests/bpf: fix broken build of test_maps
From: Daniel Borkmann @ 2017-10-21 8:23 UTC (permalink / raw)
To: Alexei Starovoitov, David S . Miller; +Cc: Chenbo Feng, Lorenzo Colitti, netdev
In-Reply-To: <20171021062533.2090015-1-ast@fb.com>
On 10/21/2017 08:25 AM, Alexei Starovoitov wrote:
> fix broken tests
>
> Fixes: e043325b3087 ("bpf: Add tests for eBPF file mode")
> Fixes: 6e71b04a8224 ("bpf: Add file mode configuration into bpf maps")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> Chenbo,
>
> this was very broken patch submission.
> Please make sure to actually compile your tests before sending patches.
Urks, and still after that many rounds ...
Acked-by: Daniel Borkmann <daniel@iogearbox.net>
^ permalink raw reply
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: Eric Dumazet @ 2017-10-21 7:45 UTC (permalink / raw)
To: Xin Long
Cc: Eric Dumazet, David Miller, network dev, Marcelo Ricardo Leitner,
Sabrina Dubroca
In-Reply-To: <CADvbK_c22nQ628LnxhdQabkbNNiuYxtYEwjTs6ToiWj4sYuBmQ@mail.gmail.com>
On Sat, 2017-10-21 at 14:51 +0800, Xin Long wrote:
> On Sat, Oct 21, 2017 at 2:18 PM, Eric Dumazet <edumazet@google.com> wrote:
> > On Fri, Oct 20, 2017 at 11:06 PM, Xin Long <lucien.xin@gmail.com> wrote:
> >>
> >>
> >> On Sat, Oct 21, 2017 at 9:27 AM, David Miller <davem@davemloft.net> wrote:
> >>>
> >>> From: Xin Long <lucien.xin@gmail.com>
> >>> Date: Thu, 19 Oct 2017 15:32:23 +0800
> >>>
> >>> > This patch is to void the potential security issue that the family
> >>> > or protocol modules are autoloaded when requesting _diag module by
> >>> > not requesting _diag module if the family or protocol is not added
> >>> > or registered in sock_diag and inet_diag.
> >>> >
> >>> > As the repost of the patch '[PATCH net] sock_diag: request _diag
> >>> > module only when the family or proto has been registered', this
> >>> > patchset fixes the compiling errors when INET is not set, and
> >>> > also split into two patches to make it clear to review.
> >>>
> >>> This makes no sense to me.
> >>>
> >>> Any user can just open a socket() in the appropriate protocol
> >>> family to cause the module to be loaded.
> >>>
> >>> If someone wants modules to not be loaded, block them using
> >>> traditional module loading infrastructure mechanisms. Or
> >>> don't load the module at all.
> >>>
> >>> Sorry I am not applying this.
> >>
> >> Hi David,
> >>
> >> I'm still thinking it's not good after 'ss', sctp, dccp,
> >> af_packet ... are just loaded, in which case, no one actually
> >> open any socket with these family or proto.
> >>
> >> I talked with Marcelo before, one scenario as he said:
> >>
> >> Imagine a customer generates a sosreport on their system, and
> >> with that, it loads sctp module. From then on, if their firewall
> >> doesn't block incoming packets for sctp, they may be prone to some
> >> remotely triggerable issue on sctp code, without even actually using
> >> sctp.
> >
> > For that reason, we have disabled autoloading of SCTP.
> > ( removing the
> > MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132");
> > MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132");
> > )
> > root must modprobe the module before it is accessible.
> >
> > However inet_diag is a way to have the module loaded anyway.
> >
> > This is why I like your patch Xin.
> >
> > David is only saying that your patch alone is not enough to prevent a
> > user to use socket() to autoload SCTP.
> Using socket() to autoload SCTP should be fine, cause users would
> use SCTP, no ?
>
> "ss" doesn't mean users intend to use SCTP, "ss" may make users
> not aware that SCTP module would be loaded, unlike socket(SCTP).
Your changelog mentions a security issue.
How have you prevented user using socket() to bypass your 'security'
feature ?
If you have not yet, this security claim is simply false.
^ permalink raw reply
* [PATCH] stmmac: Don't access tx_q->dirty_tx before netif_tx_lock
From: Bernd Edlinger @ 2017-10-21 6:51 UTC (permalink / raw)
To: Giuseppe Cavallaro, Alexandre Torgue
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org
This is the possible reason for different hard to reproduce
problems on my ARMv7-SMP test system.
The symptoms are in recent kernels imprecise external aborts,
and in older kernels various kinds of network stalls and
unexpected page allocation failures.
My testing indicates that the trouble started between v4.5 and v4.6
and prevails up to v4.14.
Using the dirty_tx before acquiring the spin lock is clearly
wrong and was first introduced with v4.6.
Fixes: e3ad57c96715 ("stmmac: review RX/TX ring management")
Signed-off-by: Bernd Edlinger <bernd.edlinger@hotmail.de>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 1763e48..c8e280fa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1800,12 +1800,13 @@ static void stmmac_tx_clean(struct stmmac_priv *priv, u32 queue)
{
struct stmmac_tx_queue *tx_q = &priv->tx_queue[queue];
unsigned int bytes_compl = 0, pkts_compl = 0;
- unsigned int entry = tx_q->dirty_tx;
+ unsigned int entry;
netif_tx_lock(priv->dev);
priv->xstats.tx_clean++;
+ entry = tx_q->dirty_tx;
while (entry != tx_q->cur_tx) {
struct sk_buff *skb = tx_q->tx_skbuff[entry];
struct dma_desc *p;
--
1.9.1
^ permalink raw reply related
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: Xin Long @ 2017-10-21 6:51 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, network dev, Marcelo Ricardo Leitner,
Sabrina Dubroca
In-Reply-To: <CANn89i+n=MMUDdjDm4SQfEfQvFM+PgSKCrtDzBY=OiwsiyvYMg@mail.gmail.com>
On Sat, Oct 21, 2017 at 2:18 PM, Eric Dumazet <edumazet@google.com> wrote:
> On Fri, Oct 20, 2017 at 11:06 PM, Xin Long <lucien.xin@gmail.com> wrote:
>>
>>
>> On Sat, Oct 21, 2017 at 9:27 AM, David Miller <davem@davemloft.net> wrote:
>>>
>>> From: Xin Long <lucien.xin@gmail.com>
>>> Date: Thu, 19 Oct 2017 15:32:23 +0800
>>>
>>> > This patch is to void the potential security issue that the family
>>> > or protocol modules are autoloaded when requesting _diag module by
>>> > not requesting _diag module if the family or protocol is not added
>>> > or registered in sock_diag and inet_diag.
>>> >
>>> > As the repost of the patch '[PATCH net] sock_diag: request _diag
>>> > module only when the family or proto has been registered', this
>>> > patchset fixes the compiling errors when INET is not set, and
>>> > also split into two patches to make it clear to review.
>>>
>>> This makes no sense to me.
>>>
>>> Any user can just open a socket() in the appropriate protocol
>>> family to cause the module to be loaded.
>>>
>>> If someone wants modules to not be loaded, block them using
>>> traditional module loading infrastructure mechanisms. Or
>>> don't load the module at all.
>>>
>>> Sorry I am not applying this.
>>
>> Hi David,
>>
>> I'm still thinking it's not good after 'ss', sctp, dccp,
>> af_packet ... are just loaded, in which case, no one actually
>> open any socket with these family or proto.
>>
>> I talked with Marcelo before, one scenario as he said:
>>
>> Imagine a customer generates a sosreport on their system, and
>> with that, it loads sctp module. From then on, if their firewall
>> doesn't block incoming packets for sctp, they may be prone to some
>> remotely triggerable issue on sctp code, without even actually using
>> sctp.
>
> For that reason, we have disabled autoloading of SCTP.
> ( removing the
> MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132");
> MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132");
> )
> root must modprobe the module before it is accessible.
>
> However inet_diag is a way to have the module loaded anyway.
>
> This is why I like your patch Xin.
>
> David is only saying that your patch alone is not enough to prevent a
> user to use socket() to autoload SCTP.
Using socket() to autoload SCTP should be fine, cause users would
use SCTP, no ?
"ss" doesn't mean users intend to use SCTP, "ss" may make users
not aware that SCTP module would be loaded, unlike socket(SCTP).
^ permalink raw reply
* [PATCH net-next] selftests/bpf: fix broken build of test_maps
From: Alexei Starovoitov @ 2017-10-21 6:25 UTC (permalink / raw)
To: David S . Miller; +Cc: Daniel Borkmann, Chenbo Feng, Lorenzo Colitti, netdev
fix broken tests
Fixes: e043325b3087 ("bpf: Add tests for eBPF file mode")
Fixes: 6e71b04a8224 ("bpf: Add file mode configuration into bpf maps")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
Chenbo,
this was very broken patch submission.
Please make sure to actually compile your tests before sending patches.
---
tools/include/uapi/linux/bpf.h | 35 ++++++++++++++++++++++++++++++---
tools/testing/selftests/bpf/test_maps.c | 6 +++---
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index fa93033dc521..d83f95ea6a1b 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -218,6 +218,10 @@ enum bpf_attach_type {
#define BPF_OBJ_NAME_LEN 16U
+/* Flags for accessing BPF object */
+#define BPF_F_RDONLY (1U << 3)
+#define BPF_F_WRONLY (1U << 4)
+
union bpf_attr {
struct { /* anonymous struct used by BPF_MAP_CREATE command */
__u32 map_type; /* one of enum bpf_map_type */
@@ -260,6 +264,7 @@ union bpf_attr {
struct { /* anonymous struct used by BPF_OBJ_* commands */
__aligned_u64 pathname;
__u32 bpf_fd;
+ __u32 file_flags;
};
struct { /* anonymous struct used by BPF_PROG_ATTACH/DETACH commands */
@@ -287,6 +292,7 @@ union bpf_attr {
__u32 map_id;
};
__u32 next_id;
+ __u32 open_flags;
};
struct { /* anonymous struct used by BPF_OBJ_GET_INFO_BY_FD */
@@ -366,7 +372,7 @@ union bpf_attr {
* jump into another BPF program
* @ctx: context pointer passed to next program
* @prog_array_map: pointer to map which type is BPF_MAP_TYPE_PROG_ARRAY
- * @index: index inside array that selects specific program to run
+ * @index: 32-bit index inside array that selects specific program to run
* Return: 0 on success or negative error
*
* int bpf_clone_redirect(skb, ifindex, flags)
@@ -642,6 +648,21 @@ union bpf_attr {
* @xdp_md: pointer to xdp_md
* @delta: An positive/negative integer to be added to xdp_md.data_meta
* Return: 0 on success or negative on error
+ *
+ * int bpf_perf_event_read_value(map, flags, buf, buf_size)
+ * read perf event counter value and perf event enabled/running time
+ * @map: pointer to perf_event_array map
+ * @flags: index of event in the map or bitmask flags
+ * @buf: buf to fill
+ * @buf_size: size of the buf
+ * Return: 0 on success or negative error code
+ *
+ * int bpf_perf_prog_read_value(ctx, buf, buf_size)
+ * read perf prog attached perf event counter and enabled/running time
+ * @ctx: pointer to ctx
+ * @buf: buf to fill
+ * @buf_size: size of the buf
+ * Return : 0 on success or negative error code
*/
#define __BPF_FUNC_MAPPER(FN) \
FN(unspec), \
@@ -744,7 +765,9 @@ enum bpf_func_id {
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
#define BPF_F_DONT_FRAGMENT (1ULL << 2)
-/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
+/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
+ * BPF_FUNC_perf_event_read_value flags.
+ */
#define BPF_F_INDEX_MASK 0xffffffffULL
#define BPF_F_CURRENT_CPU BPF_F_INDEX_MASK
/* BPF_FUNC_perf_event_output for sk_buff input context. */
@@ -872,7 +895,7 @@ struct bpf_prog_info {
__u32 created_by_uid;
__u32 nr_map_ids;
__aligned_u64 map_ids;
- char name[BPF_OBJ_NAME_LEN];
+ char name[BPF_OBJ_NAME_LEN];
} __attribute__((aligned(8)));
struct bpf_map_info {
@@ -937,4 +960,10 @@ enum {
#define TCP_BPF_IW 1001 /* Set TCP initial congestion window */
#define TCP_BPF_SNDCWND_CLAMP 1002 /* Set sndcwnd_clamp */
+struct bpf_perf_event_value {
+ __u64 counter;
+ __u64 enabled;
+ __u64 running;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 896f23cfe918..c302847671dc 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -1035,7 +1035,7 @@ static void test_map_parallel(void)
static void test_map_rdonly(void)
{
- int i, fd, key = 0, value = 0;
+ int fd, key = 0, value = 0;
fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
MAP_SIZE, map_flags | BPF_F_RDONLY);
@@ -1058,7 +1058,7 @@ static void test_map_rdonly(void)
static void test_map_wronly(void)
{
- int i, fd, key = 0, value = 0;
+ int fd, key = 0, value = 0;
fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value),
MAP_SIZE, map_flags | BPF_F_WRONLY);
@@ -1071,7 +1071,7 @@ static void test_map_wronly(void)
key = 1;
value = 1234;
/* Insert key=1 element. */
- assert(bpf_map_update_elem(fd, &key, &value, BPF_ANY) == 0)
+ assert(bpf_map_update_elem(fd, &key, &value, BPF_ANY) == 0);
/* Check that key=2 is not found. */
assert(bpf_map_lookup_elem(fd, &key, &value) == -1 && errno == EPERM);
--
2.9.5
^ permalink raw reply related
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: Eric Dumazet @ 2017-10-21 6:18 UTC (permalink / raw)
To: Xin Long
Cc: David Miller, network dev, Marcelo Ricardo Leitner,
Sabrina Dubroca
In-Reply-To: <CADvbK_fWmmC3ggpoT--Pxk3GxZ8Gq_rbdFGTuXk-BuTHTO=eXw@mail.gmail.com>
On Fri, Oct 20, 2017 at 11:06 PM, Xin Long <lucien.xin@gmail.com> wrote:
>
>
> On Sat, Oct 21, 2017 at 9:27 AM, David Miller <davem@davemloft.net> wrote:
>>
>> From: Xin Long <lucien.xin@gmail.com>
>> Date: Thu, 19 Oct 2017 15:32:23 +0800
>>
>> > This patch is to void the potential security issue that the family
>> > or protocol modules are autoloaded when requesting _diag module by
>> > not requesting _diag module if the family or protocol is not added
>> > or registered in sock_diag and inet_diag.
>> >
>> > As the repost of the patch '[PATCH net] sock_diag: request _diag
>> > module only when the family or proto has been registered', this
>> > patchset fixes the compiling errors when INET is not set, and
>> > also split into two patches to make it clear to review.
>>
>> This makes no sense to me.
>>
>> Any user can just open a socket() in the appropriate protocol
>> family to cause the module to be loaded.
>>
>> If someone wants modules to not be loaded, block them using
>> traditional module loading infrastructure mechanisms. Or
>> don't load the module at all.
>>
>> Sorry I am not applying this.
>
> Hi David,
>
> I'm still thinking it's not good after 'ss', sctp, dccp,
> af_packet ... are just loaded, in which case, no one actually
> open any socket with these family or proto.
>
> I talked with Marcelo before, one scenario as he said:
>
> Imagine a customer generates a sosreport on their system, and
> with that, it loads sctp module. From then on, if their firewall
> doesn't block incoming packets for sctp, they may be prone to some
> remotely triggerable issue on sctp code, without even actually using
> sctp.
For that reason, we have disabled autoloading of SCTP.
( removing the
MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132");
MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132");
)
root must modprobe the module before it is accessible.
However inet_diag is a way to have the module loaded anyway.
This is why I like your patch Xin.
David is only saying that your patch alone is not enough to prevent a
user to use socket() to autoload SCTP.
^ permalink raw reply
* [PATCH] net: can: dev: remove unused code in can_restart
From: Gustavo A. R. Silva @ 2017-10-21 3:36 UTC (permalink / raw)
To: Wolfgang Grandegger, Marc Kleine-Budde
Cc: linux-can, netdev, linux-kernel, Gustavo A. R. Silva
Value assigned to variable err is overwritten at line
562: err = priv->do_set_mode(dev, CAN_MODE_START); before
it can be used.
Also, notice that this code has been there since 2014.
Addresses-Coverity-ID: 1227031
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
This code was tested by compilation only.
drivers/net/can/dev.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 365a8cc..064d0f6 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -543,10 +543,9 @@ static void can_restart(struct net_device *dev)
/* send restart message upstream */
skb = alloc_can_err_skb(dev, &cf);
- if (skb == NULL) {
- err = -ENOMEM;
+ if (!skb)
goto restart;
- }
+
cf->can_id |= CAN_ERR_RESTARTED;
netif_rx(skb);
--
2.7.4
^ permalink raw reply related
* Re: [net-next v2 3/4] openvswitch: Add meter infrastructure
From: Pravin Shelar @ 2017-10-21 3:32 UTC (permalink / raw)
To: Andy Zhou; +Cc: Greg Rose, Joe Stringer, Linux Kernel Network Developers
In-Reply-To: <CABKoBm1JdL3K=oG-xn7kc31t_CHM-EtnexQngoyrGQ8PUZitSg@mail.gmail.com>
On Thu, Oct 19, 2017 at 5:58 PM, Andy Zhou <azhou@ovn.org> wrote:
>
> On Thu, Oct 19, 2017 at 02:47 Pravin Shelar <pshelar@ovn.org> wrote:
>>
>> On Tue, Oct 17, 2017 at 12:36 AM, Andy Zhou <azhou@ovn.org> wrote:
>> > OVS kernel datapath so far does not support Openflow meter action.
>> > This is the first stab at adding kernel datapath meter support.
>> > This implementation supports only drop band type.
>> >
>> > Signed-off-by: Andy Zhou <azhou@ovn.org>
>> > ---
>> > net/openvswitch/Makefile | 1 +
>> > net/openvswitch/datapath.c | 14 +-
>> > net/openvswitch/datapath.h | 3 +
>> > net/openvswitch/meter.c | 604
>> > +++++++++++++++++++++++++++++++++++++++++++++
>> > net/openvswitch/meter.h | 54 ++++
>> > 5 files changed, 674 insertions(+), 2 deletions(-)
>> > create mode 100644 net/openvswitch/meter.c
>> > create mode 100644 net/openvswitch/meter.h
>> >
>> This patch mostly looks good. I have one comment below.
>>
>> > +static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info
>> > *info)
>> > +{
>> > + struct nlattr **a = info->attrs;
>> > + struct dp_meter *meter, *old_meter;
>> > + struct sk_buff *reply;
>> > + struct ovs_header *ovs_reply_header;
>> > + struct ovs_header *ovs_header = info->userhdr;
>> > + struct datapath *dp;
>> > + int err;
>> > + u32 meter_id;
>> > + bool failed;
>> > +
>> > + meter = dp_meter_create(a);
>> > + if (IS_ERR_OR_NULL(meter))
>> > + return PTR_ERR(meter);
>> > +
>> > + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
>> > + &ovs_reply_header);
>> > + if (IS_ERR(reply)) {
>> > + err = PTR_ERR(reply);
>> > + goto exit_free_meter;
>> > + }
>> > +
>> > + ovs_lock();
>> > + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
>> > + if (!dp) {
>> > + err = -ENODEV;
>> > + goto exit_unlock;
>> > + }
>> > +
>> > + if (!a[OVS_METER_ATTR_ID]) {
>> > + err = -ENODEV;
>> > + goto exit_unlock;
>> > + }
>> > +
>> > + meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
>> > +
>> > + /* Cannot fail after this. */
>> > + old_meter = lookup_meter(dp, meter_id);
>> I do not see RCU read lock taken here. This is not correctness issue
>> but it could cause RCU checker to spit out warning message. You could
>> do same trick that is done in get_dp() to avoid this issue.
>
> O.K.
>>
>>
>>
>> Can you also test the code with rcu sparse check config option enabled?
>
>
> Do you mean to sparse compile with CONFIG_PROVE_LOCKING and
> CONFIG_DENUG_OBJECTS_RCU_HEAD?
You could use all following options simultaneously:
CONFIG_PREEMPT
CONFIG_DEBUG_PREEMPT
CONFIG_DEBUG_SPINLOCK
CONFIG_DEBUG_ATOMIC_SLEEP
CONFIG_PROVE_RCU
CONFIG_DEBUG_OBJECTS_RCU_HEAD
Thanks,
Pravin.
^ permalink raw reply
* Re: [patch net-next v2 00/20] net: sched: convert cls ndo_setup_tc offload calls to per-block callbacks
From: David Miller @ 2017-10-21 2:04 UTC (permalink / raw)
To: jiri
Cc: netdev, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, jeffrey.t.kirsher, saeedm,
matanb, leonro, idosch, jakub.kicinski, ast, daniel, simon.horman,
pieter.jansenvanvuuren, john.hurley, alexander.h.duyck
In-Reply-To: <20171019135048.4306-1-jiri@resnulli.us>
From: Jiri Pirko <jiri@resnulli.us>
Date: Thu, 19 Oct 2017 15:50:28 +0200
> This patchset is a bit bigger, but most of the patches are doing the
> same changes in multiple classifiers and drivers. I could do some
> squashes, but I think it is better split.
>
> This is another dependency on the way to shared block implementation.
> The goal is to remove use of tp->q in classifiers code.
>
> Also, this provides drivers possibility to track binding of blocks to
> qdiscs. Legacy drivers which do not support shared block offloading.
> register one callback per binding. That maintains the current
> functionality we have with ndo_setup_tc. Drivers which support block
> sharing offload register one callback per block which safes overhead.
>
> Patches 1-4 introduce the binding notifications and per-block callbacks
> Patches 5-8 add block callbacks calls to classifiers
> Patches 9-17 do convert from ndo_setup_tc calls to block callbacks for
> classifier offloads in drivers
> Patches 18-20 do cleanup
Series applied.
^ permalink raw reply
* Re: [PATCH net] geneve: Fix function matching VNI and tunnel ID on big-endian
From: David Miller @ 2017-10-21 1:51 UTC (permalink / raw)
To: sbrivio; +Cc: netdev, pshelar, girish.moodalbail, sd
In-Reply-To: <7fb32cd5779c5c0133754194aa6914fa8689853e.1508412016.git.sbrivio@redhat.com>
From: Stefano Brivio <sbrivio@redhat.com>
Date: Thu, 19 Oct 2017 13:31:28 +0200
> On big-endian machines, functions converting between tunnel ID
> and VNI use the three LSBs of tunnel ID storage to map VNI.
>
> The comparison function eq_tun_id_and_vni(), on the other hand,
> attempted to map the VNI from the three MSBs. Fix it by using
> the same check implemented on LE, which maps VNI from the three
> LSBs of tunnel ID.
>
> Fixes: 2e0b26e10352 ("geneve: Optimize geneve device lookup.")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> ---
> I guess this should be queued up for -stable, back to 4.10
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: pull-request: can 2017-10-19
From: David Miller @ 2017-10-21 1:30 UTC (permalink / raw)
To: mkl; +Cc: netdev, linux-can, kernel
In-Reply-To: <20171019111945.13580-1-mkl@pengutronix.de>
From: Marc Kleine-Budde <mkl@pengutronix.de>
Date: Thu, 19 Oct 2017 13:19:34 +0200
> this is a pull request of 11 patches for the upcoming 4.14 release.
>
> There are 6 patches by ZHU Yi for the flexcan driver, that work around
> the CAN error handling state transition problems found in various
> incarnations of the flexcan IP core.
>
> The patch by Colin Ian King fixes a potential NULL pointer deref in the
> CAN broad cast manager (bcm). One patch by me replaces a direct deref of a RCU
> protected pointer by rcu_access_pointer. My second patch adds missing
> OOM error handling in af_can. A patch by Stefan Mätje for the esd_usb2
> driver fixes the dlc in received RTR frames. And the last patch is by
> Wolfgang Grandegger, it fixes a busy loop in the gs_usb driver in case
> it runs out of TX contexts.
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH net 0/2] net: diag: fix a potential security issue
From: David Miller @ 2017-10-21 1:27 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, edumazet, marcelo.leitner, sd
In-Reply-To: <cover.1508398111.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Thu, 19 Oct 2017 15:32:23 +0800
> This patch is to void the potential security issue that the family
> or protocol modules are autoloaded when requesting _diag module by
> not requesting _diag module if the family or protocol is not added
> or registered in sock_diag and inet_diag.
>
> As the repost of the patch '[PATCH net] sock_diag: request _diag
> module only when the family or proto has been registered', this
> patchset fixes the compiling errors when INET is not set, and
> also split into two patches to make it clear to review.
This makes no sense to me.
Any user can just open a socket() in the appropriate protocol
family to cause the module to be loaded.
If someone wants modules to not be loaded, block them using
traditional module loading infrastructure mechanisms. Or
don't load the module at all.
Sorry I am not applying this.
^ permalink raw reply
* Re: [PATCH net-next] ipv6: let trace_fib6_table_lookup() dereference the fib table
From: David Miller @ 2017-10-21 1:23 UTC (permalink / raw)
To: pabeni; +Cc: netdev, rostedt, mingo, dsa
In-Reply-To: <48235df8fdf787356070d8baa1ac9816ef5736d4.1508398120.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 19 Oct 2017 09:31:43 +0200
> The perf traces for ipv6 routing code show a relevant cost around
> trace_fib6_table_lookup(), even if no trace is enabled. This is
> due to the fib6_table de-referencing currently performed by the
> caller.
>
> Let's the tracing code pay this overhead, passing to the trace
> helper the table pointer. This gives small but measurable
> performance improvement under UDP flood.
>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Looks good, applied.
^ permalink raw reply
* Re: pull request: bluetooth-next 2017-10-19
From: David Miller @ 2017-10-21 1:22 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth, netdev
In-Reply-To: <20171019062914.GA9702@x1c.lan>
From: Johan Hedberg <johan.hedberg@gmail.com>
Date: Thu, 19 Oct 2017 09:29:14 +0300
> Here's the first bluetooth-next pull request targeting the 4.15 kernel
> release.
>
> - Multiple fixes & improvements to the hci_bcm driver
> - DT improvements, e.g. new local-bd-address property
> - Fixes & improvements to ECDH usage. Private key is now generated by
> the crypto subsystem.
> - gcc-4.9 warning fixes
>
> Please let me know if there are any issues pulling. Thanks.
Pulled, thanks Johan.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox