* Re: [PATCH net] sctp: fix an use-after-free issue in sctp_sock_dump
From: David Miller @ 2017-09-15 21:48 UTC (permalink / raw)
To: lucien.xin; +Cc: netdev, linux-sctp, marcelo.leitner, nhorman, pabeni
In-Reply-To: <9de7bef8f4f8c0f369361118f6d2daee5d188467.1505444541.git.lucien.xin@gmail.com>
From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 15 Sep 2017 11:02:21 +0800
> Commit 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the
> dump") tried to fix an use-after-free issue by checking !sctp_sk(sk)->ep
> with holding sock and sock lock.
>
> But Paolo noticed that endpoint could be destroyed in sctp_rcv without
> sock lock protection. It means the use-after-free issue still could be
> triggered when sctp_rcv put and destroy ep after sctp_sock_dump checks
> !ep, although it's pretty hard to reproduce.
>
> I could reproduce it by mdelay in sctp_rcv while msleep in sctp_close
> and sctp_sock_dump long time.
>
> This patch is to add another param cb_done to sctp_for_each_transport
> and dump ep->assocs with holding tsp after jumping out of transport's
> traversal in it to avoid this issue.
>
> It can also improve sctp diag dump to make it run faster, as no need
> to save sk into cb->args[5] and keep calling sctp_for_each_transport
> any more.
>
> This patch is also to use int * instead of int for the pos argument
> in sctp_for_each_transport, which could make postion increment only
> in sctp_for_each_transport and no need to keep changing cb->args[2]
> in sctp_sock_filter and sctp_sock_dump any more.
>
> Fixes: 86fdb3448cc1 ("sctp: ensure ep is not destroyed before doing the dump")
> Reported-by: Paolo Abeni <pabeni@redhat.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] netvsc: increase default receive buffer size
From: David Miller @ 2017-09-15 21:48 UTC (permalink / raw)
To: stephen; +Cc: netdev, haiyangz, sthemmin, devel
In-Reply-To: <20170914163107.8404-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Thu, 14 Sep 2017 09:31:07 -0700
> The default receive buffer size was reduced by recent change
> to a value which was appropriate for 10G and Windows Server 2016.
> But the value is too small for full performance with 40G on Azure.
> Increase the default back to maximum supported by host.
>
> Fixes: 8b5327975ae1 ("netvsc: allow controlling send/recv buffer size")
> Signed-off-by: Stephen Hemminger <sthemmin@microsoft.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] tcp: update skb->skb_mstamp more carefully
From: David Miller @ 2017-09-15 21:37 UTC (permalink / raw)
To: eric.dumazet
Cc: liujian56, edumazet, ycheng, hkchu, netdev, weiyongjun1,
wangkefeng.wang
In-Reply-To: <1505359839.15310.209.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 13 Sep 2017 20:30:39 -0700
> From: Eric Dumazet <edumazet@googl.com>
>
> liujian reported a problem in TCP_USER_TIMEOUT processing with a patch
> in tcp_probe_timer() :
> https://www.spinics.net/lists/netdev/msg454496.html
>
> After investigations, the root cause of the problem is that we update
> skb->skb_mstamp of skbs in write queue, even if the attempt to send a
> clone or copy of it failed. One reason being a routing problem.
>
> This patch prevents this, solving liujian issue.
>
> It also removes a potential RTT miscalculation, since
> __tcp_retransmit_skb() is not OR-ing TCP_SKB_CB(skb)->sacked with
> TCPCB_EVER_RETRANS if a failure happens, but skb->skb_mstamp has
> been changed.
>
> A future ACK would then lead to a very small RTT sample and min_rtt
> would then be lowered to this too small value.
>
> Tested:
...
> Signed-off-by: Eric Dumazet <edumazet@googl.com>
> Reported-by: liujian <liujian56@huawei.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment
From: Joe Perches @ 2017-09-15 21:31 UTC (permalink / raw)
To: Srishti Sharma, samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel
In-Reply-To: <9380189edb2722544981c868699ade29ab5c297d.1505508423.git.srishtishar@gmail.com>
On Sat, 2017-09-16 at 02:36 +0530, Srishti Sharma wrote:
> Parentheses are not needed on the right hand side of assignment
> statement in most cases. Done using the following semantic
> patch by coccinelle.
[]
> @@
> identifier E,F,G,f;
> expression e,r;
> @@
>
> (
> E = (G == F);
> >
>
> E = (e == r);
> >
>
> E =
> -(
> ...
> -)
> ;
> )
[]
> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
[]
> @@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> if (mcs->new_speed <= 115200) {
> rval &= ~MCS_FIR;
>
> - rst = (mcs->speed > 115200);
> + rst = mcs->speed > 115200;
> if (rst)
> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
Coccinelle is a good tool, but its output is limited to
the correctness
and completeness of its input script.
Please look at the suggested modifications of the script
and examine the code for other similar uses.
The else if block immediately below this is:
} else if (mcs->new_speed <= 1152000) {
rval &= ~MCS_FIR;
if ((rst = !(mcs->speed == 576000 || mcs->speed == 11520
00)))
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
which should also be corrected by this patch.
^ permalink raw reply
* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: Florian Fainelli @ 2017-09-15 21:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev, edumazet, jqiaoulk
In-Reply-To: <20170915.142511.221689318302073484.davem@davemloft.net>
On September 15, 2017 2:25:11 PM PDT, David Miller <davem@davemloft.net> wrote:
>From: Florian Fainelli <f.fainelli@gmail.com>
>Date: Tue, 12 Sep 2017 13:14:26 -0700
>
>> We can enter a deadlock situation because there is no sufficient
>protection
>> when ndo_get_stats64() runs in process context to guard against RX or
>TX NAPI
>> contexts running in softirq, this can lead to the following lockdep
>splat and
>> actual deadlock was experienced as well with an iperf session in the
>background
>> and a while loop doing ifconfig + ethtool.
> ...
>> So just remove the u64_stats_update_begin()/end() pair in
>ndo_get_stats64()
>> since it does not appear to be useful for anything. No inconsistency
>was
>> observed with either ifconfig or ethtool, global TX counts equal the
>sum of
>> per-queue TX counts on a 32-bit architecture.
>>
>> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>
>Applied.
FYI, there is another patch needed to ensure consistency between ethtool reported stats and netdevice stats, will submit that after some more testing. Thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH] net: ipv4: fix l3slave check for index returned in IP_PKTINFO
From: David Miller @ 2017-09-15 21:28 UTC (permalink / raw)
To: dsahern; +Cc: netdev
In-Reply-To: <1505347897-27422-1-git-send-email-dsahern@gmail.com>
From: David Ahern <dsahern@gmail.com>
Date: Wed, 13 Sep 2017 17:11:37 -0700
> rt_iif is only set to the actual egress device for the output path. The
> recent change to consider the l3slave flag when returning IP_PKTINFO
> works for local traffic (the correct device index is returned), but it
> broke the more typical use case of packets received from a remote host
> always returning the VRF index rather than the original ingress device.
> Update the fixup to consider l3slave and rt_iif actually getting set.
>
> Fixes: 1dfa76390bf05 ("net: ipv4: add check for l3slave for index returned in IP_PKTINFO")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied, thanks David.
^ permalink raw reply
* Re: [PATCH v2] net: smsc911x: Quieten netif during suspend
From: David Miller @ 2017-09-15 21:26 UTC (permalink / raw)
To: geert+renesas
Cc: steve.glendinning, andrew, f.fainelli, netdev, linux-pm,
linux-renesas-soc, linux-kernel
In-Reply-To: <1505324525-9998-1-git-send-email-geert+renesas@glider.be>
From: Geert Uytterhoeven <geert+renesas@glider.be>
Date: Wed, 13 Sep 2017 19:42:05 +0200
> If the network interface is kept running during suspend, the net core
> may call net_device_ops.ndo_start_xmit() while the Ethernet device is
> still suspended, which may lead to a system crash.
>
> E.g. on sh73a0/kzm9g and r8a73a4/ape6evm, the external Ethernet chip is
> driven by a PM controlled clock. If the Ethernet registers are accessed
> while the clock is not running, the system will crash with an imprecise
> external abort.
>
> As this is a race condition with a small time window, it is not so easy
> to trigger at will. Using pm_test may increase your chances:
>
> # echo 0 > /sys/module/printk/parameters/console_suspend
> # echo platform > /sys/power/pm_test
> # echo mem > /sys/power/state
>
> To fix this, make sure the network interface is quietened during
> suspend.
>
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
Applied.
^ permalink raw reply
* Re: [PATCH net] net: systemport: Fix 64-bit stats deadlock
From: David Miller @ 2017-09-15 21:25 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev, edumazet, jqiaoulk
In-Reply-To: <1505247266-42195-1-git-send-email-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 12 Sep 2017 13:14:26 -0700
> We can enter a deadlock situation because there is no sufficient protection
> when ndo_get_stats64() runs in process context to guard against RX or TX NAPI
> contexts running in softirq, this can lead to the following lockdep splat and
> actual deadlock was experienced as well with an iperf session in the background
> and a while loop doing ifconfig + ethtool.
...
> So just remove the u64_stats_update_begin()/end() pair in ndo_get_stats64()
> since it does not appear to be useful for anything. No inconsistency was
> observed with either ifconfig or ethtool, global TX counts equal the sum of
> per-queue TX counts on a 32-bit architecture.
>
> Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: vrf: avoid gcc-4.6 warning
From: David Miller @ 2017-09-15 21:22 UTC (permalink / raw)
To: arnd; +Cc: dsa, shm, ja, mschiffer, weiwan, netdev, linux-kernel
In-Reply-To: <20170912201100.3680915-1-arnd@arndb.de>
From: Arnd Bergmann <arnd@arndb.de>
Date: Tue, 12 Sep 2017 22:10:53 +0200
> When building an allmodconfig kernel with gcc-4.6, we get a rather
> odd warning:
>
> drivers/net/vrf.c: In function ‘vrf_ip6_input_dst’:
> drivers/net/vrf.c:964:3: error: initialized field with side-effects overwritten [-Werror]
> drivers/net/vrf.c:964:3: error: (near initialization for ‘fl6’) [-Werror]
>
> I have no idea what this warning is even trying to say, but it does
> seem like a false positive. Reordering the initialization in to match
> the structure definition gets rid of the warning, and might also avoid
> whatever gcc thinks is wrong here.
>
> Fixes: 9ff74384600a ("net: vrf: Handle ipv6 multicast and link-local addresses")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
That's extremely weird, I can't explain it either.
Oh well, applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Vincent Bernat @ 2017-09-15 21:08 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, bridge, netdev
In-Reply-To: <20170915193808.22338-1-vincent@bernat.im>
❦ 15 septembre 2017 21:38 +0200, Vincent Bernat <vincent@bernat.im> :
> Currently, when an interface is released from a bridge, we get a
> RTM_DELLINK event through netlink:
>
> Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
> link/ether 6e:23:c2:54:3a:b3
It should be noted this only happens when using the ioctl API. When
using the netlink API, rtnetlink.c will send a RTM_NEWLINK because of
the modification.
--
When in doubt, tell the truth.
-- Mark Twain
^ permalink raw reply
* Re: [PATCH] VSOCK: fix uapi/linux/vm_sockets.h incomplete types
From: David Miller @ 2017-09-15 21:14 UTC (permalink / raw)
To: stefanha; +Cc: netdev, jhansen
In-Reply-To: <20170912163435.4049-1-stefanha@redhat.com>
From: Stefan Hajnoczi <stefanha@redhat.com>
Date: Tue, 12 Sep 2017 17:34:35 +0100
> This patch fixes the following compiler errors when userspace
> applications use the vm_sockets.h header:
>
> include/uapi/linux/vm_sockets.h:148:32: error: invalid application of ‘sizeof’ to incomplete type ‘struct sockaddr’
> unsigned char svm_zero[sizeof(struct sockaddr) -
> ^~~~~~
> include/uapi/linux/vm_sockets.h:149:18: error: ‘sa_family_t’ undeclared here (not in a function)
> sizeof(sa_family_t) -
> ^~~~~~~~~~~
>
> Two issues:
> 1. In the kernel struct sockaddr comes in via <linux/socket.h> but in
> userspace <sys/socket.h> is required.
> 2. struct sockaddr_vm has a __kernel_sa_family_t field so let's be
> consistent and use the same type for the sizeof(sa_family_t)
> calculation.
>
> Currently userspace applications work around this broken header by first
> including <sys/socket.h>. In the kernel there is no compiler error
> because <linux/socket.h> provides everything. It's worth fixing the
> header file though.
>
> Cc: Jorgen Hansen <jhansen@vmware.com>
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
> include/uapi/linux/vm_sockets.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/vm_sockets.h b/include/uapi/linux/vm_sockets.h
> index b4ed5d895699..4ae5c625ac56 100644
> --- a/include/uapi/linux/vm_sockets.h
> +++ b/include/uapi/linux/vm_sockets.h
> @@ -18,6 +18,10 @@
>
> #include <linux/socket.h>
>
> +#ifndef __KERNEL__
> +#include <sys/socket.h> /* struct sockaddr */
> +#endif
> +
There is no precedence whatsoever to include sys/socket.h in _any_ UAPI
header file provided by the kernel.
__kernel_sa_family_t is what should be used in UAPI headers, only
non-UAPI headers can use plain sa_family_t.
So that is the correct fix for this problem.
Thank you.
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 21:14 UTC (permalink / raw)
To: Julia Lawall
Cc: devel, samuel, Greg KH, Linux kernel mailing list,
outreachy-kernel, netdev
In-Reply-To: <alpine.DEB.2.20.1709152308300.2533@hadrien>
On Sat, Sep 16, 2017 at 2:39 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 16 Sep 2017, Srishti Sharma wrote:
>
>> On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>> >
>> >
>> > On Sat, 16 Sep 2017, Srishti Sharma wrote:
>> >
>> >> Write assignment statement outside the if statement. Done using
>> >> the following semantic patch by coccinelle.
>> >>
>> >> @@
>> >> identifier E;
>> >> expression F;
>> >> statement S;
>> >> @@
>> >>
>> >> -if((E = F))
>> >> +E = F;
>> >> +if(E)
>> >> S
>> >>
>> >> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
>> >
>> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>>
>> Have sent a patchset for this instead .
>
> Thanks. Actually, you should have copied the ack.
Oh.. Sorry didn't realise I could do that :)
Regards,
Srishti
>
> julia
>
>>
>> Regards,
>> Srishti
>> >
>> >> ---
>> >> Changes in v2:
>> >> -Semicolon was missing in one of the statements of the
>> >> semantic patch
>> >>
>> >> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>> >> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
>> >> drivers/staging/irda/net/irqueue.c | 3 ++-
>> >> 3 files changed, 10 insertions(+), 6 deletions(-)
>> >>
>> >> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
>> >> index 723e49b..82bfc05 100644
>> >> --- a/drivers/staging/irda/drivers/irda-usb.c
>> >> +++ b/drivers/staging/irda/drivers/irda-usb.c
>> >> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>> >> urb->transfer_flags = 0;
>> >>
>> >> /* Irq disabled -> GFP_ATOMIC */
>> >> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
>> >> + ret = usb_submit_urb(urb, GFP_ATOMIC);
>> >> + if (ret)
>> >> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
>> >> - }
>> >> }
>> >>
>> >> /*------------------------------------------------------------------*/
>> >> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
>> >> index c3f0b25..2b674d5 100644
>> >> --- a/drivers/staging/irda/drivers/mcs7780.c
>> >> +++ b/drivers/staging/irda/drivers/mcs7780.c
>> >> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>> >> if (mcs->new_speed <= 115200) {
>> >> rval &= ~MCS_FIR;
>> >>
>> >> - if ((rst = (mcs->speed > 115200)))
>> >> + rst = (mcs->speed > 115200);
>> >> + if (rst)
>> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>> >>
>> >> } else if (mcs->new_speed <= 1152000) {
>> >> rval &= ~MCS_FIR;
>> >>
>> >> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
>> >> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
>> >> + if (rst)
>> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>> >>
>> >> } else {
>> >> rval |= MCS_FIR;
>> >>
>> >> - if ((rst = (mcs->speed != 4000000)))
>> >> + rst = (mcs->speed != 4000000);
>> >> + if (rst)
>> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>> >>
>> >> }
>> >> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
>> >> index 160dc89..5aab072 100644
>> >> --- a/drivers/staging/irda/net/irqueue.c
>> >> +++ b/drivers/staging/irda/net/irqueue.c
>> >> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>> >>
>> >> while(*name) {
>> >> h = (h<<4) + *name++;
>> >> - if ((g = (h & 0xf0000000)))
>> >> + g = (h & 0xf0000000);
>> >> + if (g)
>> >> h ^=g>>24;
>> >> h &=~g;
>> >> }
>> >> --
>> >> 2.7.4
>> >>
>> >> --
>> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
>> >> For more options, visit https://groups.google.com/d/optout.
>> >>
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1%2BuUkjU%3DZ5bKTJtzUs7sA%40mail.gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
^ permalink raw reply
* Re: [PATCH net] ipv6: fix net.ipv6.conf.all interface DAD handlers
From: David Miller @ 2017-09-15 21:12 UTC (permalink / raw)
To: mcroce; +Cc: netdev, linux-doc, ek
In-Reply-To: <20170912154637.12996-1-mcroce@redhat.com>
From: Matteo Croce <mcroce@redhat.com>
Date: Tue, 12 Sep 2017 17:46:37 +0200
> Currently, writing into
> net.ipv6.conf.all.{accept_dad,use_optimistic,optimistic_dad} has no effect.
> Fix handling of these flags by:
>
> - using the maximum of global and per-interface values for the
> accept_dad flag. That is, if at least one of the two values is
> non-zero, enable DAD on the interface. If at least one value is
> set to 2, enable DAD and disable IPv6 operation on the interface if
> MAC-based link-local address was found
>
> - using the logical OR of global and per-interface values for the
> optimistic_dad flag. If at least one of them is set to one, optimistic
> duplicate address detection (RFC 4429) is enabled on the interface
>
> - using the logical OR of global and per-interface values for the
> use_optimistic flag. If at least one of them is set to one,
> optimistic addresses won't be marked as deprecated during source address
> selection on the interface.
>
> While at it, as we're modifying the prototype for ipv6_use_optimistic_addr(),
> drop inline, and let the compiler decide.
>
> Fixes: 7fd2561e4ebd ("net: ipv6: Add a sysctl to make optimistic addresses useful candidates")
> Signed-off-by: Matteo Croce <mcroce@redhat.com>
Erik, please review.
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Julia Lawall @ 2017-09-15 21:09 UTC (permalink / raw)
To: Srishti Sharma
Cc: samuel, Greg KH, netdev, devel, Linux kernel mailing list,
outreachy-kernel
In-Reply-To: <CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1+uUkjU=Z5bKTJtzUs7sA@mail.gmail.com>
On Sat, 16 Sep 2017, Srishti Sharma wrote:
> On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Sat, 16 Sep 2017, Srishti Sharma wrote:
> >
> >> Write assignment statement outside the if statement. Done using
> >> the following semantic patch by coccinelle.
> >>
> >> @@
> >> identifier E;
> >> expression F;
> >> statement S;
> >> @@
> >>
> >> -if((E = F))
> >> +E = F;
> >> +if(E)
> >> S
> >>
> >> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
> >
> > Acked-by: Julia Lawall <julia.lawall@lip6.fr>
>
> Have sent a patchset for this instead .
Thanks. Actually, you should have copied the ack.
julia
>
> Regards,
> Srishti
> >
> >> ---
> >> Changes in v2:
> >> -Semicolon was missing in one of the statements of the
> >> semantic patch
> >>
> >> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
> >> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
> >> drivers/staging/irda/net/irqueue.c | 3 ++-
> >> 3 files changed, 10 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
> >> index 723e49b..82bfc05 100644
> >> --- a/drivers/staging/irda/drivers/irda-usb.c
> >> +++ b/drivers/staging/irda/drivers/irda-usb.c
> >> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
> >> urb->transfer_flags = 0;
> >>
> >> /* Irq disabled -> GFP_ATOMIC */
> >> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> >> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> >> + if (ret)
> >> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> >> - }
> >> }
> >>
> >> /*------------------------------------------------------------------*/
> >> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> >> index c3f0b25..2b674d5 100644
> >> --- a/drivers/staging/irda/drivers/mcs7780.c
> >> +++ b/drivers/staging/irda/drivers/mcs7780.c
> >> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> >> if (mcs->new_speed <= 115200) {
> >> rval &= ~MCS_FIR;
> >>
> >> - if ((rst = (mcs->speed > 115200)))
> >> + rst = (mcs->speed > 115200);
> >> + if (rst)
> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
> >>
> >> } else if (mcs->new_speed <= 1152000) {
> >> rval &= ~MCS_FIR;
> >>
> >> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> >> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> >> + if (rst)
> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
> >>
> >> } else {
> >> rval |= MCS_FIR;
> >>
> >> - if ((rst = (mcs->speed != 4000000)))
> >> + rst = (mcs->speed != 4000000);
> >> + if (rst)
> >> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
> >>
> >> }
> >> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
> >> index 160dc89..5aab072 100644
> >> --- a/drivers/staging/irda/net/irqueue.c
> >> +++ b/drivers/staging/irda/net/irqueue.c
> >> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
> >>
> >> while(*name) {
> >> h = (h<<4) + *name++;
> >> - if ((g = (h & 0xf0000000)))
> >> + g = (h & 0xf0000000);
> >> + if (g)
> >> h ^=g>>24;
> >> h &=~g;
> >> }
> >> --
> >> 2.7.4
> >>
> >> --
> >> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> >> To post to this group, send email to outreachy-kernel@googlegroups.com.
> >> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
> >> For more options, visit https://groups.google.com/d/optout.
> >>
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAB3L5oxr5K5aMo06ROCVV9ejY6a_g1%2BuUkjU%3DZ5bKTJtzUs7sA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 21:07 UTC (permalink / raw)
To: Julia Lawall
Cc: samuel, Greg KH, netdev, devel, Linux kernel mailing list,
outreachy-kernel
In-Reply-To: <alpine.DEB.2.20.1709152200030.2533@hadrien>
On Sat, Sep 16, 2017 at 1:30 AM, Julia Lawall <julia.lawall@lip6.fr> wrote:
>
>
> On Sat, 16 Sep 2017, Srishti Sharma wrote:
>
>> Write assignment statement outside the if statement. Done using
>> the following semantic patch by coccinelle.
>>
>> @@
>> identifier E;
>> expression F;
>> statement S;
>> @@
>>
>> -if((E = F))
>> +E = F;
>> +if(E)
>> S
>>
>> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
>
> Acked-by: Julia Lawall <julia.lawall@lip6.fr>
Have sent a patchset for this instead .
Regards,
Srishti
>
>> ---
>> Changes in v2:
>> -Semicolon was missing in one of the statements of the
>> semantic patch
>>
>> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
>> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
>> drivers/staging/irda/net/irqueue.c | 3 ++-
>> 3 files changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
>> index 723e49b..82bfc05 100644
>> --- a/drivers/staging/irda/drivers/irda-usb.c
>> +++ b/drivers/staging/irda/drivers/irda-usb.c
>> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
>> urb->transfer_flags = 0;
>>
>> /* Irq disabled -> GFP_ATOMIC */
>> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
>> + ret = usb_submit_urb(urb, GFP_ATOMIC);
>> + if (ret)
>> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
>> - }
>> }
>>
>> /*------------------------------------------------------------------*/
>> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
>> index c3f0b25..2b674d5 100644
>> --- a/drivers/staging/irda/drivers/mcs7780.c
>> +++ b/drivers/staging/irda/drivers/mcs7780.c
>> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
>> if (mcs->new_speed <= 115200) {
>> rval &= ~MCS_FIR;
>>
>> - if ((rst = (mcs->speed > 115200)))
>> + rst = (mcs->speed > 115200);
>> + if (rst)
>> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>>
>> } else if (mcs->new_speed <= 1152000) {
>> rval &= ~MCS_FIR;
>>
>> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
>> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
>> + if (rst)
>> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>>
>> } else {
>> rval |= MCS_FIR;
>>
>> - if ((rst = (mcs->speed != 4000000)))
>> + rst = (mcs->speed != 4000000);
>> + if (rst)
>> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>>
>> }
>> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
>> index 160dc89..5aab072 100644
>> --- a/drivers/staging/irda/net/irqueue.c
>> +++ b/drivers/staging/irda/net/irqueue.c
>> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>>
>> while(*name) {
>> h = (h<<4) + *name++;
>> - if ((g = (h & 0xf0000000)))
>> + g = (h & 0xf0000000);
>> + if (g)
>> h ^=g>>24;
>> h &=~g;
>> }
>> --
>> 2.7.4
>>
>> --
>> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
>> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
>> To post to this group, send email to outreachy-kernel@googlegroups.com.
>> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
>> For more options, visit https://groups.google.com/d/optout.
>>
^ permalink raw reply
* [PATCH 2/2] Staging: irda: Remove parentheses on the right of assignment
From: Srishti Sharma @ 2017-09-15 21:06 UTC (permalink / raw)
To: samuel
Cc: devel, gregkh, linux-kernel, outreachy-kernel, Srishti Sharma,
netdev
In-Reply-To: <cover.1505508423.git.srishtishar@gmail.com>
Parentheses are not needed on the right hand side of assignment
statement in most cases. Done using the following semantic
patch by coccinelle.
@@
identifier E,F,G,f;
expression e,r;
@@
(
E = (G == F);
|
E = (e == r);
|
E =
-(
...
-)
;
)
Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
drivers/staging/irda/drivers/mcs7780.c | 4 ++--
drivers/staging/irda/net/irqueue.c | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index 2b674d5..d52e9f4 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -605,7 +605,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
if (mcs->new_speed <= 115200) {
rval &= ~MCS_FIR;
- rst = (mcs->speed > 115200);
+ rst = mcs->speed > 115200;
if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
@@ -619,7 +619,7 @@ static int mcs_speed_change(struct mcs_cb *mcs)
} else {
rval |= MCS_FIR;
- rst = (mcs->speed != 4000000);
+ rst = mcs->speed != 4000000;
if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 5aab072..14291cb 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -217,7 +217,7 @@ static __u32 hash( const char* name)
while(*name) {
h = (h<<4) + *name++;
- g = (h & 0xf0000000);
+ g = h & 0xf0000000;
if (g)
h ^=g>>24;
h &=~g;
--
2.7.4
^ permalink raw reply related
* [PATCH 1/2] Staging: irda: Don't use assignment inside if statement
From: Srishti Sharma @ 2017-09-15 21:05 UTC (permalink / raw)
To: samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
Srishti Sharma
In-Reply-To: <cover.1505508423.git.srishtishar@gmail.com>
Write assignment statement outside of the if statement. Done
using the following semantic patch by coccinelle.
@@
identifier E;
expression F;
statement S;
@@
-if((E = F))
+E = F;
+if(E)
S
Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
---
drivers/staging/irda/drivers/irda-usb.c | 4 ++--
drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
drivers/staging/irda/net/irqueue.c | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
index 723e49b..82bfc05 100644
--- a/drivers/staging/irda/drivers/irda-usb.c
+++ b/drivers/staging/irda/drivers/irda-usb.c
@@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
urb->transfer_flags = 0;
/* Irq disabled -> GFP_ATOMIC */
- if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+ if (ret)
net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
- }
}
/*------------------------------------------------------------------*/
diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
index c3f0b25..2b674d5 100644
--- a/drivers/staging/irda/drivers/mcs7780.c
+++ b/drivers/staging/irda/drivers/mcs7780.c
@@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
if (mcs->new_speed <= 115200) {
rval &= ~MCS_FIR;
- if ((rst = (mcs->speed > 115200)))
+ rst = (mcs->speed > 115200);
+ if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
} else if (mcs->new_speed <= 1152000) {
rval &= ~MCS_FIR;
- if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
+ rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
+ if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
} else {
rval |= MCS_FIR;
- if ((rst = (mcs->speed != 4000000)))
+ rst = (mcs->speed != 4000000);
+ if (rst)
mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
}
diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
index 160dc89..5aab072 100644
--- a/drivers/staging/irda/net/irqueue.c
+++ b/drivers/staging/irda/net/irqueue.c
@@ -217,7 +217,8 @@ static __u32 hash( const char* name)
while(*name) {
h = (h<<4) + *name++;
- if ((g = (h & 0xf0000000)))
+ g = (h & 0xf0000000);
+ if (g)
h ^=g>>24;
h &=~g;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 0/2] Assign outside if
From: Srishti Sharma @ 2017-09-15 21:05 UTC (permalink / raw)
To: samuel
Cc: gregkh, netdev, devel, linux-kernel, outreachy-kernel,
Srishti Sharma
This patch series intends to remove the assignment statements
inside the if statement, and eliminates the cases of parentheses
around the right hand side of assignment generated as a result of
the same.
Srishti Sharma (2):
Staging: irda: Don't use assignment inside if statement
Staging: irda: Remove parentheses on the right of assignment
drivers/staging/irda/drivers/irda-usb.c | 4 ++--
drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
drivers/staging/irda/net/irqueue.c | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: [PATCH] qed: remove unnecessary call to memset
From: David Miller @ 2017-09-15 21:02 UTC (permalink / raw)
To: himanshujha199640
Cc: Yuval.Mintz, Ariel.Elior, everest-linux-l2, netdev, linux-kernel
In-Reply-To: <1505215162-3917-1-git-send-email-himanshujha199640@gmail.com>
From: Himanshu Jha <himanshujha199640@gmail.com>
Date: Tue, 12 Sep 2017 16:49:22 +0530
> call to memset to assign 0 value immediately after allocating
> memory with kzalloc is unnecesaary as kzalloc allocates the memory
> filled with 0 value.
>
> Semantic patch used to resolve this issue:
>
> @@
> expression e,e2; constant c;
> statement S;
> @@
>
> e = kzalloc(e2, c);
> if(e == NULL) S
> - memset(e, 0, e2);
>
> Signed-off-by: Himanshu Jha <himanshujha199640@gmail.com>
Applied.
^ permalink raw reply
* Re: Use after free in __dst_destroy_metrics_generic
From: Eric Dumazet @ 2017-09-15 21:00 UTC (permalink / raw)
To: Cong Wang
Cc: Subash Abhinov Kasiviswanathan, Linux Kernel Network Developers,
Lorenzo Colitti
In-Reply-To: <CAM_iQpXKfUkbyuuZEZXyPrVVCoPaxnCmCARz=Jf0VtU1m9NsZg@mail.gmail.com>
On Fri, 2017-09-08 at 09:10 -0700, Cong Wang wrote:
> On Thu, Sep 7, 2017 at 5:52 PM, Subash Abhinov Kasiviswanathan
> <subashab@codeaurora.org> wrote:
> > We are seeing a possible use after free in ip6_dst_destroy.
> >
> > It appears as if memory of the __DST_METRICS_PTR(old) was freed in some path
> > and allocated
> > to ion driver. ion driver has also freed it. Finally the memory is freed by
> > the
> > fib gc and crashes since it is already deallocated.
>
> Does the attach (compile-only) patch help anything?
>
> From my _quick_ glance, it seems we miss the refcnt'ing
> right in __dst_destroy_metrics_generic().
>
> Thanks!
Hi Cong
I believe your patch makes a lot of sense, please submit it formally ?
Thanks !
^ permalink raw reply
* Re: [PATCH net] l2tp: fix race condition in l2tp_tunnel_delete
From: Guillaume Nault @ 2017-09-15 20:38 UTC (permalink / raw)
To: Sabrina Dubroca; +Cc: Tom Parkin, netdev, Xin Long
In-Reply-To: <20170915145502.GA18157@bistromath.localdomain>
On Fri, Sep 15, 2017 at 04:55:02PM +0200, Sabrina Dubroca wrote:
> 2017-09-15, 10:42:59 +0100, Tom Parkin wrote:
> > On Fri, Sep 15, 2017 at 11:08:07AM +0200, Sabrina Dubroca wrote:
> > > The tunnel is currently removed from the list during destruction. This
> > > can lead to a double-free of the struct sock if we try to delete the tunnel
> > > twice fast enough.
> > >
> > > The first delete operation does a lookup (l2tp_tunnel_get), finds the
> > > tunnel, calls l2tp_tunnel_delete, which queues it for deletion by
> > > l2tp_tunnel_del_work.
> > >
> > > The second delete operation also finds the tunnel and calls
> > > l2tp_tunnel_delete. If the workqueue has already fired and started
> > > running l2tp_tunnel_del_work, then l2tp_tunnel_delete will queue the
> > > same tunnel a second time, and try to free the socket again.
> > >
> > > Add a dead flag and remove tunnel from its list earlier. Then we can
> > > remove the check of queue_work's result that was meant to prevent that
> > > race but doesn't.
> >
> > How do we avoid leaving stale information on the tunnel list for
> > use-cases which don't delete tunnels using netlink? For example the
> > L2TPv2 ppp/socket API depends on sk_destruct to clean up the kernel
> > context on socket destruction. Similarly, userspace may just close
> > the tunnel socket without first making netlink calls to delete the
> > tunnel.
> >
> > By moving the tunnel list removal from l2tp_tunnel_destruct to
> > l2tp_tunnel_delete I can't see how codepaths which don't involve
> > l2tp_tunnel_delete don't end up with a corrupted tunnel list.
>
> Ok, thanks for pointing that out. We could go with just the ->dead
> flag then.
>
Yes, I guess that's the least instrusive way to fix this issue.
> I'm not sure whether we need to set it in l2tp_tunnel_destruct as
> well.
>
It shouldn't be necessary. If the socket is managed by userspace, then
l2tp_tunnel_del_work() gets it using sockfd_lookup(). Therefore it
shouldn't find it if userspace is in the process of releasing it.
Also l2tp_tunnel_del_work() doesn't try to release sockets handled by
userspace, so not checking ->dead in l2tp_tunnel_destruct() should be
safe.
^ permalink raw reply
* [PATCH iproute2] Add information about COLORFGBG to ip.8 man page
From: Roland Hopferwieser @ 2017-09-15 20:04 UTC (permalink / raw)
To: netdev
diff --git a/man/man8/ip.8 b/man/man8/ip.8
index ae018fdf..2a27a56e 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -187,7 +187,8 @@ executes specified command over all objects, it
depends if command supports this
.TP
.BR "\-c" , " -color"
-Use color output.
+Use color output. The color palette is affected by the COLORFGBG
environment variable, which typically has the form "fg;bg".
+If "bg" is set to 0-6 or 8, the dark color palette is used.
.TP
.BR "\-t" , " \-timestamp"
^ permalink raw reply related
* Re: [PATCH net] packet: hold bind lock when rebinding to fanout hook
From: Willem de Bruijn @ 2017-09-15 20:01 UTC (permalink / raw)
To: Cong Wang
Cc: Willem de Bruijn, Linux Kernel Network Developers, David Miller,
nixiaoming
In-Reply-To: <CAM_iQpXOS3cB8GtuVZaQq7AnFkHQXxcBqkg0azMU-JpMtv4NRw@mail.gmail.com>
>> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
>> index c26172995511..d288f52c53f7 100644
>> --- a/net/packet/af_packet.c
>> +++ b/net/packet/af_packet.c
>> @@ -1684,10 +1684,6 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>>
>> mutex_lock(&fanout_mutex);
>>
>> - err = -EINVAL;
>> - if (!po->running)
>> - goto out;
>> -
>> err = -EALREADY;
>> if (po->fanout)
>> goto out;
>> @@ -1749,7 +1745,10 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>> list_add(&match->list, &fanout_list);
>> }
>> err = -EINVAL;
>> - if (match->type == type &&
>> +
>> + spin_lock(&po->bind_lock);
>> + if (po->running &&
>
>
> As you move the po->running check later after setting po->rollover, I wonder
> if po->rollover possibly depends on po>running on other path?
The rollover code does not explicitly check po->running, if that is
what you are concerned about.
A newly allocated po->rollover structure will also only be accessed
from the datapath once the socket is added to a fanout group. Both
before and after this patch, that happens well after the po->running
check.
If the socket already had a po->rollover, then it must already have
had a po->fanout, too, so fanout_add does not reach this code.
>> + match->type == type &&
>> match->prot_hook.type == po->prot_hook.type &&
>> match->prot_hook.dev == po->prot_hook.dev) {
>> err = -ENOSPC;
>> @@ -1761,6 +1760,13 @@ static int fanout_add(struct sock *sk, u16 id, u16 type_flags)
>> err = 0;
>> }
>> }
>> + spin_unlock(&po->bind_lock);
>> +
>> + if (err && !refcount_read(&match->sk_ref)) {
>> + list_del(&match->list);
>> + kfree(match);
>> + }
>
> This looks correct but still seems odd, it smells you don't use refcnt in an
> expected way.
It tests whether the object has no references, in which case it must have
been newly allocated and the fanout join operation must have failed.
I don't see an obviously simpler patch.
The entire code could perhaps be restructured eventually. There is,
for instance, no reason to test match->{type, prot_hook.type, prot_hook.dev}
when having just allocated the structure. Nor to test whether sk_ref exceeds
PACKET_FANOUT_MAX. Conversely, this sk_ref == 0 test only makes sense
when having taken the (!match) branch earlier.
But a refactor is out of scope for a bug fix.
^ permalink raw reply
* Re: [Outreachy kernel] [PATCH v2] Staging: irda: Don't use assignment inside if statement
From: Julia Lawall @ 2017-09-15 20:00 UTC (permalink / raw)
To: Srishti Sharma
Cc: samuel, gregkh, netdev, devel, linux-kernel, outreachy-kernel
In-Reply-To: <1505504680-22167-1-git-send-email-srishtishar@gmail.com>
On Sat, 16 Sep 2017, Srishti Sharma wrote:
> Write assignment statement outside the if statement. Done using
> the following semantic patch by coccinelle.
>
> @@
> identifier E;
> expression F;
> statement S;
> @@
>
> -if((E = F))
> +E = F;
> +if(E)
> S
>
> Signed-off-by: Srishti Sharma <srishtishar@gmail.com>
Acked-by: Julia Lawall <julia.lawall@lip6.fr>
> ---
> Changes in v2:
> -Semicolon was missing in one of the statements of the
> semantic patch
>
> drivers/staging/irda/drivers/irda-usb.c | 4 ++--
> drivers/staging/irda/drivers/mcs7780.c | 9 ++++++---
> drivers/staging/irda/net/irqueue.c | 3 ++-
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/staging/irda/drivers/irda-usb.c b/drivers/staging/irda/drivers/irda-usb.c
> index 723e49b..82bfc05 100644
> --- a/drivers/staging/irda/drivers/irda-usb.c
> +++ b/drivers/staging/irda/drivers/irda-usb.c
> @@ -334,9 +334,9 @@ static void irda_usb_change_speed_xbofs(struct irda_usb_cb *self)
> urb->transfer_flags = 0;
>
> /* Irq disabled -> GFP_ATOMIC */
> - if ((ret = usb_submit_urb(urb, GFP_ATOMIC))) {
> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> + if (ret)
> net_warn_ratelimited("%s(), failed Speed URB\n", __func__);
> - }
> }
>
> /*------------------------------------------------------------------*/
> diff --git a/drivers/staging/irda/drivers/mcs7780.c b/drivers/staging/irda/drivers/mcs7780.c
> index c3f0b25..2b674d5 100644
> --- a/drivers/staging/irda/drivers/mcs7780.c
> +++ b/drivers/staging/irda/drivers/mcs7780.c
> @@ -605,19 +605,22 @@ static int mcs_speed_change(struct mcs_cb *mcs)
> if (mcs->new_speed <= 115200) {
> rval &= ~MCS_FIR;
>
> - if ((rst = (mcs->speed > 115200)))
> + rst = (mcs->speed > 115200);
> + if (rst)
> mcs_set_reg(mcs, MCS_MINRXPW_REG, 0);
>
> } else if (mcs->new_speed <= 1152000) {
> rval &= ~MCS_FIR;
>
> - if ((rst = !(mcs->speed == 576000 || mcs->speed == 1152000)))
> + rst = !(mcs->speed == 576000 || mcs->speed == 1152000);
> + if (rst)
> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
> } else {
> rval |= MCS_FIR;
>
> - if ((rst = (mcs->speed != 4000000)))
> + rst = (mcs->speed != 4000000);
> + if (rst)
> mcs_set_reg(mcs, MCS_MINRXPW_REG, 5);
>
> }
> diff --git a/drivers/staging/irda/net/irqueue.c b/drivers/staging/irda/net/irqueue.c
> index 160dc89..5aab072 100644
> --- a/drivers/staging/irda/net/irqueue.c
> +++ b/drivers/staging/irda/net/irqueue.c
> @@ -217,7 +217,8 @@ static __u32 hash( const char* name)
>
> while(*name) {
> h = (h<<4) + *name++;
> - if ((g = (h & 0xf0000000)))
> + g = (h & 0xf0000000);
> + if (g)
> h ^=g>>24;
> h &=~g;
> }
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/1505504680-22167-1-git-send-email-srishtishar%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* [PATCH net-next v1] bridge: also trigger RTM_NEWLINK when interface is released from bridge
From: Vincent Bernat @ 2017-09-15 19:38 UTC (permalink / raw)
To: Stephen Hemminger, David S. Miller, bridge, netdev; +Cc: Vincent Bernat
Currently, when an interface is released from a bridge, we get a
RTM_DELLINK event through netlink:
Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 6e:23:c2:54:3a:b3
Userspace has to interpret that as a removal from the bridge, not as a
complete removal of the interface. When an bridged interface is
completely removed, we get two events:
Deleted 2: dummy0: <BROADCAST,NOARP> mtu 1500 master bridge0 state DOWN
link/ether 6e:23:c2:54:3a:b3
Deleted 2: dummy0: <BROADCAST,NOARP> mtu 1500 qdisc noop state DOWN group default
link/ether 6e:23:c2:54:3a:b3 brd ff:ff:ff:ff:ff:ff
In constrast, when an interface is released from a bond, we get a
RTM_NEWLINK with only the new characteristics (no master):
3: dummy1: <BROADCAST,NOARP,SLAVE,UP,LOWER_UP> mtu 1500 qdisc noqueue master bond0 state UNKNOWN group default
link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
3: dummy1: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
4: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
3: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN group default
link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
3: dummy1: <BROADCAST,NOARP> mtu 1500 qdisc noqueue state DOWN group default
link/ether ca:c8:7b:66:f8:25 brd ff:ff:ff:ff:ff:ff
4: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP group default
link/ether ae:dc:7a:8c:9a:3c brd ff:ff:ff:ff:ff:ff
Userland may be confused by the fact we say a link is deleted while
its characteristics are only modified. A first solution would have
been to turn the RTM_DELLINK event in del_nbp() into a RTM_NEWLINK
event. However, maybe some piece of userland is relying on this
RTM_DELLINK to detect when a bridged interface is released. Instead,
we also emit a RTM_NEWLINK event once the interface is
released (without master info).
Deleted 2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 master bridge0 state UNKNOWN
link/ether 8a:bb:e7:94:b1:f8
2: dummy0: <BROADCAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default
link/ether 8a:bb:e7:94:b1:f8 brd ff:ff:ff:ff:ff:ff
Signed-off-by: Vincent Bernat <vincent@bernat.im>
---
net/bridge/br_if.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_if.c b/net/bridge/br_if.c
index f3aef22931ab..636e0a842f8a 100644
--- a/net/bridge/br_if.c
+++ b/net/bridge/br_if.c
@@ -288,6 +288,8 @@ static void del_nbp(struct net_bridge_port *p)
dev->priv_flags &= ~IFF_BRIDGE_PORT;
+ rtmsg_ifinfo(RTM_NEWLINK, dev, IFF_MASTER, GFP_KERNEL);
+
netdev_rx_handler_unregister(dev);
br_multicast_del_port(p);
--
2.14.1
^ permalink raw reply related
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