* Re: Kernel memory corruption in CIPSO labeled TCP packets processing.
From: Nazarov Sergey @ 2019-01-24 14:46 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module@vger.kernel.org, selinux@vger.kernel.org,
netdev@vger.kernel.org, Casey Schaufler
In-Reply-To: <CAHC9VhRhEvcU804qMfZik7qwohuCDfcbvJMHRT+DxMPvvMDEGQ@mail.gmail.com>
22.01.2019, 20:48, "Paul Moore" <paul@paul-moore.com>:
>
> Yes, exactly. If you don't pass the skb it shouldn't attempt to call
> icmp_send() in case of error.
>
> --
> paul moore
> www.paul-moore.com
You are right, sorry. We can do that without ip_options_compile modification.
Simplified patch 2:
---
net/ipv4/cipso_ipv4.c | 18 ++++++++++++++++--
1 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b..a4ed0a4 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1735,13 +1735,27 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
*/
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
{
+ unsigned char optbuf[sizeof(struct ip_options) + 40];
+ struct ip_options *opt = (struct ip_options *)optbuf;
+
if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
return;
+ /*
+ * We might be called above the IP layer,
+ * so we can not use icmp_send and IPCB here.
+ */
+
+ memset(opt, 0, sizeof(struct ip_options));
+ opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
+ memcpy(opt->__data, (unsigned char *)&(ip_hdr(skb)[1]), opt->optlen);
+ if (ip_options_compile(dev_net(skb->dev), opt, NULL))
+ return;
+
if (gateway)
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
else
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
}
/**
^ permalink raw reply related
* atheros: atl2: replace dev_kfree_skb_any() by dev_consume_skb_any()
From: Yang Wei @ 2019-01-24 14:45 UTC (permalink / raw)
To: netdev; +Cc: jcliburn, chris.snook, davem, yang.wei9, Yang Wei
atl2_xmit_frame() should call dev_consume_skb_any() when the
transmission is successful. It makes drop profiles more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/atheros/atlx/atl2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index bb41bec..c44e952 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -909,7 +909,7 @@ static netdev_tx_t atl2_xmit_frame(struct sk_buff *skb,
(adapter->txd_write_ptr >> 2));
mmiowb();
- dev_kfree_skb_any(skb);
+ dev_consume_skb_any(skb);
return NETDEV_TX_OK;
}
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net 5/5] net: aquantia: added err var into AQ_HW_WAIT_FOR construct
From: Igor Russkikh @ 2019-01-24 14:28 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David S . Miller, netdev@vger.kernel.org, Nikita Danilov
In-Reply-To: <20190123131502.GA26413@lunn.ch>
> You can define a little helper:
>
> static u32 aq_hw_read_mpi_state2_addr(struct aq_hw_s *self)
> {
> return aq_hw_read_reg(self, HW_ATL_FW2X_MPI_STATE2_ADDR);
> }
>
> Then
>
> readx_poll_timeout(u32 aq_hw_read_mpi_state2_addr, self,
> stats_val, orig_stats_val != stats_val & BIT(CAPS_HI_STATISTICS));
>
[cut]
> You see this sort of construct in quite a lot of drivers.
Thanks, Andrew,
Think that way this'll be the optimal case with best readability.
We'll update the patch.
Regards, Igor
^ permalink raw reply
* Re: [PATCH 3/7] net/ethernet: Add parse_protocol header_ops support
From: Willem de Bruijn @ 2019-01-24 14:21 UTC (permalink / raw)
To: Maxim Mikityanskiy
Cc: David S. Miller, Saeed Mahameed, Willem de Bruijn, Jason Wang,
Eric Dumazet, netdev@vger.kernel.org, Eran Ben Elisha,
Tariq Toukan
In-Reply-To: <AM6PR05MB5879862AB004334C0B85C875D19A0@AM6PR05MB5879.eurprd05.prod.outlook.com>
On Thu, Jan 24, 2019 at 4:48 AM Maxim Mikityanskiy <maximmi@mellanox.com> wrote:
>
> > -----Original Message-----
> > From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Sent: 23 January, 2019 16:15
> > To: Maxim Mikityanskiy <maximmi@mellanox.com>
> > Cc: David S. Miller <davem@davemloft.net>; Saeed Mahameed
> > <saeedm@mellanox.com>; Willem de Bruijn <willemb@google.com>; Jason Wang
> > <jasowang@redhat.com>; Eric Dumazet <edumazet@google.com>;
> > netdev@vger.kernel.org; Eran Ben Elisha <eranbe@mellanox.com>; Tariq Toukan
> > <tariqt@mellanox.com>
> > Subject: Re: [PATCH 3/7] net/ethernet: Add parse_protocol header_ops support
> >
> > On Mon, Jan 14, 2019 at 8:21 AM Maxim Mikityanskiy <maximmi@mellanox.com>
> > wrote:
> > >
> > > The previous commit introduced parse_protocol callback which should
> > > extract the protocol number from the L2 header. Make all Ethernet
> > > devices support it.
> > >
> > > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
> > > ---
> > > include/linux/etherdevice.h | 1 +
> > > net/ethernet/eth.c | 13 +++++++++++++
> > > 2 files changed, 14 insertions(+)
> > >
> > > diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
> > > index 2c0af7b00715..e2f3b21cd72a 100644
> > > --- a/include/linux/etherdevice.h
> > > +++ b/include/linux/etherdevice.h
> > > @@ -44,6 +44,7 @@ int eth_header_cache(const struct neighbour *neigh,
> > struct hh_cache *hh,
> > > __be16 type);
> > > void eth_header_cache_update(struct hh_cache *hh, const struct net_device
> > *dev,
> > > const unsigned char *haddr);
> > > +__be16 eth_header_parse_protocol(const struct sk_buff *skb);
> >
> > Does not need to be exposed in the header file or exported.
>
> Are you sure? All the other Ethernet header_ops callbacks are exported
> and declared in the header. I'm not sure about the reason why it is done
> in such a way, but my guess is that it will be useful if some driver
> decides to replace one callback in header_ops but to use the default
> ones for the rest of callbacks.
I don't exactly follow this. But I think that many are exported
because Ethernet is so common that of these are also called directly
instead of through header_ops. Looking at other header_ops
implementations, or other such callback structs, shows many examples
where the members are static local functions.
> If the default callbacks were not
> accessible externally, it wouldn't be possible. So, I'm following the
> existing pattern here, and there are reasons for that. What do you think
> about it?
^ permalink raw reply
* Re: [PATCH 1/7] net: Don't set transport offset to invalid value
From: Willem de Bruijn @ 2019-01-24 14:19 UTC (permalink / raw)
To: Maxim Mikityanskiy
Cc: David S. Miller, Saeed Mahameed, Willem de Bruijn, Jason Wang,
Eric Dumazet, netdev@vger.kernel.org, Eran Ben Elisha,
Tariq Toukan
In-Reply-To: <AM6PR05MB5879E1A2149359E1613A69EAD19A0@AM6PR05MB5879.eurprd05.prod.outlook.com>
On Thu, Jan 24, 2019 at 4:48 AM Maxim Mikityanskiy <maximmi@mellanox.com> wrote:
>
> > > but the general idea is that we
> > > report this status, so if you say that my version is also good for you,
> > > I'll leave it as is. It was just a rationale for my decision.
> >
> > It's fine. But please avoid the code churn in xenvif_tx_submit
> > with to extra indentation. This is equivalent:
> >
> > - if (skb_is_gso(skb)) {
> > + if (skb_is_gso(skb) && th_set) {
> >
> > More fundamentally, the code has the assumption that th_set
> > always holds if skb_is_gso(skb). Why add the check? This is
> > another example that the return value is not really needed.
>
> What about
>
> if (skb_is_gso(skb)) {
> BUG_ON(!skb_transport_header_was_set(skb));
>
> ?
Good idea to validate. Please on error use WARN_ON_ONCE and free the
packet as with other errors in this function. BUG_ONs are problematic
when a path to them is found.
The other option is to stay as close to the current logic as possible
in this driver and set the mac header at offset 0 if probe fails.
Either through the return value or skb_transport_header_was_set.
> I think it's cleaner than skipping the action here if dissect failed and
> propagating a potential bug further.
>
> > > > If this is the only reason for the boolean return value, using
> > > > skb_transport_header_was_set() is more standard (I immediately know
> > > > what's happening when I read it), slightly less code change and avoids
> > > > introducing a situation where the majority of callers ignore a return
> > > > value. I think it's preferable. But these merits are certainly
> > > > debatable, so either is fine.
> > >
> > > From my side, I wanted to avoid calling skb_transport_header_was_set
> > > twice, so I made skb_try_probe_transport_header return whether it
> > > succeeded or not. I think "try" in the function name indicates this idea
> > > pretty clearly. This result status is pretty useful, it just happened
> > > that it's not needed in many places,
> >
> > Which is an indication that it's perhaps not needed.
>
> Well, from the point of view of the function, it looks reasonable to
> notify the caller whether the call was successful or not... You know,
> many functions return error codes.
>
> However, this case is rather special, because it turned out that we
> don't actually need that error status immediately, but it can be
> requested much later, and we already have skb_transport_header_was_set
> for that.
>
> So, considering these points, the return value can be removed, as all
> use cases are "probe now, check later", not "probe now and handle errors
> immediately".
^ permalink raw reply
* Re: [PATCH] sfp: sfp_read: split-up request when hw rx buffer is too small.
From: Andrew Lunn @ 2019-01-24 14:12 UTC (permalink / raw)
To: René van Dorst
Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190124140323.Horde.-BXNAw3tfYO0-oHPatZeOOi@www.vdorst.com>
> >>+ /* Many i2c hw have limited rx buffers, split-up request when needed. */
> >>+ while ((q->max_read_len) && (len > q->max_read_len)) {
> >>+ ret = sfp->read(sfp, a2, addr, buf, q->max_read_len);
> >
> >Hi René
> >
> >I think you want to pass MIN(len, q->max_read_len) to read().
>
> Hi Andrew,
>
> max_read_len is 0 when there is no quirk.
> I can write it a bit differently depending on the outcome of my other email.
Hi René
No, you misunderstood me.
> >>+ ret = sfp->read(sfp, a2, addr, buf, q->max_read_len);
Say max_read_len = 64
The SFP code asks to read 68 bytes. The first call to read() is going
to ask for 64 bytes. The second call is going to also ask for 64
bytes, writing 60 bytes passed the end of buf. Bad things then happen.
>
> >>+ if (ret < 0)
> >>+ return ret;
> >>+ rx_bytes += ret;
> >>+ addr += q->max_read_len;
> >>+ buf += q->max_read_len;
> >>+ len -= q->max_read_len;
> >
> >I would prefer you add ret, not q->max_read_len. There is a danger it
> >returned less bytes than you asked for.
>
> Getting less bytes then asked is already an error I think.
> I could check the return size and directly return the number of bytes that I
> have. The callers are checking for size and they can retry if wanted. So that
> should not be an issue.
If that is true, why is rx_bytes += ret, where as all the others are
+= q->max_read_len. Please be consistent. The general pattern of a
read function in POSIX systems is that it returns how many bytes were
actually returned. So i would always use += ret.
> By reading the SSF spec we can write to a user writable EERPOM area of 120
> bytes.
> But the current code has only has 1 sfp_write for a byte value.
>
> So for now I should say no.
So how about adding a WARN_ON. If the request is bigger than what the
quirk allows, make it very obvious we have an issue.
Andrew
^ permalink raw reply
* [PATCH iproute2-next] iproute: lwtunnel set ip tunnel flags
From: wenxu @ 2019-01-24 14:03 UTC (permalink / raw)
To: dsahern; +Cc: netdev
From: wenxu <wenxu@ucloud.cn>
ip l add dev tun type gretap external
ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 dev gretap
For gretap example when the command set the id but don't set the
TUNNEL_KEY flags. There is no key field in the send packet
User can set flags with key, csum, seq
ip r a 10.0.0.1 encap ip dst 192.168.152.171 id 1000 key csum dev gretap
Signed-off-by: wenxu <wenxu@ucloud.cn>
---
ip/iproute_lwtunnel.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++-
man/man8/ip-route.8.in | 3 ++-
2 files changed, 59 insertions(+), 2 deletions(-)
diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c
index aee18ac..03217b8 100644
--- a/ip/iproute_lwtunnel.c
+++ b/ip/iproute_lwtunnel.c
@@ -31,7 +31,7 @@
#include <linux/seg6_iptunnel.h>
#include <linux/seg6_hmac.h>
#include <linux/seg6_local.h>
-#include <net/if.h>
+#include <linux/if_tunnel.h>
static const char *format_encap_type(int type)
{
@@ -294,6 +294,7 @@ static void print_encap_mpls(FILE *fp, struct rtattr *encap)
static void print_encap_ip(FILE *fp, struct rtattr *encap)
{
struct rtattr *tb[LWTUNNEL_IP_MAX+1];
+ __u16 flags;
parse_rtattr_nested(tb, LWTUNNEL_IP_MAX, encap);
@@ -318,6 +319,16 @@ static void print_encap_ip(FILE *fp, struct rtattr *encap)
if (tb[LWTUNNEL_IP_TOS])
print_uint(PRINT_ANY, "tos",
"tos %d ", rta_getattr_u8(tb[LWTUNNEL_IP_TOS]));
+
+ if (tb[LWTUNNEL_IP_FLAGS]) {
+ flags = rta_getattr_u16(tb[LWTUNNEL_IP_FLAGS]);
+ if (flags & TUNNEL_KEY)
+ print_bool(PRINT_ANY, "key", "key ", true);
+ if (flags & TUNNEL_CSUM)
+ print_bool(PRINT_ANY, "csum", "csum ", true);
+ if (flags & TUNNEL_SEQ)
+ print_bool(PRINT_ANY, "seq", "seq ", true);
+ }
}
static void print_encap_ila(FILE *fp, struct rtattr *encap)
@@ -354,6 +365,7 @@ static void print_encap_ila(FILE *fp, struct rtattr *encap)
static void print_encap_ip6(FILE *fp, struct rtattr *encap)
{
struct rtattr *tb[LWTUNNEL_IP6_MAX+1];
+ __u16 flags;
parse_rtattr_nested(tb, LWTUNNEL_IP6_MAX, encap);
@@ -379,6 +391,16 @@ static void print_encap_ip6(FILE *fp, struct rtattr *encap)
if (tb[LWTUNNEL_IP6_TC])
print_uint(PRINT_ANY, "tc",
"tc %u ", rta_getattr_u8(tb[LWTUNNEL_IP6_TC]));
+
+ if (tb[LWTUNNEL_IP6_FLAGS]) {
+ flags = rta_getattr_u16(tb[LWTUNNEL_IP6_FLAGS]);
+ if (flags & TUNNEL_KEY)
+ print_bool(PRINT_ANY, "key", "key ", true);
+ if (flags & TUNNEL_CSUM)
+ print_bool(PRINT_ANY, "csum", "csum ", true);
+ if (flags & TUNNEL_SEQ)
+ print_bool(PRINT_ANY, "seq", "seq ", true);
+ }
}
static void print_encap_bpf(FILE *fp, struct rtattr *encap)
@@ -777,9 +799,11 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
int *argcp, char ***argvp)
{
int id_ok = 0, dst_ok = 0, src_ok = 0, tos_ok = 0, ttl_ok = 0;
+ int key_ok = 0, csum_ok = 0, seq_ok = 0;
char **argv = *argvp;
int argc = *argcp;
int ret = 0;
+ __u16 flags = 0;
while (argc > 0) {
if (strcmp(*argv, "id") == 0) {
@@ -827,6 +851,18 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
if (get_u8(&ttl, *argv, 0))
invarg("\"ttl\" value is invalid\n", *argv);
ret = rta_addattr8(rta, len, LWTUNNEL_IP_TTL, ttl);
+ } else if (strcmp(*argv, "key") == 0) {
+ if (key_ok++)
+ duparg2("key", *argv);
+ flags |= TUNNEL_KEY;
+ } else if (strcmp(*argv, "csum") == 0) {
+ if (csum_ok++)
+ duparg2("csum", *argv);
+ flags |= TUNNEL_CSUM;
+ } else if (strcmp(*argv, "seq") == 0) {
+ if (seq_ok++)
+ duparg2("seq", *argv);
+ flags |= TUNNEL_SEQ;
} else {
break;
}
@@ -835,6 +871,9 @@ static int parse_encap_ip(struct rtattr *rta, size_t len,
argc--; argv++;
}
+ if (flags)
+ ret = rta_addattr16(rta, len, LWTUNNEL_IP_FLAGS, flags);
+
/* argv is currently the first unparsed argument,
* but the lwt_parse_encap() caller will move to the next,
* so step back
@@ -927,9 +966,11 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
int *argcp, char ***argvp)
{
int id_ok = 0, dst_ok = 0, src_ok = 0, tos_ok = 0, ttl_ok = 0;
+ int key_ok = 0, csum_ok = 0, seq_ok = 0;
char **argv = *argvp;
int argc = *argcp;
int ret = 0;
+ __u16 flags = 0;
while (argc > 0) {
if (strcmp(*argv, "id") == 0) {
@@ -979,6 +1020,18 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
*argv);
ret = rta_addattr8(rta, len, LWTUNNEL_IP6_HOPLIMIT,
hoplimit);
+ } else if (strcmp(*argv, "key") == 0) {
+ if (key_ok++)
+ duparg2("key", *argv);
+ flags |= TUNNEL_KEY;
+ } else if (strcmp(*argv, "csum") == 0) {
+ if (csum_ok++)
+ duparg2("csum", *argv);
+ flags |= TUNNEL_CSUM;
+ } else if (strcmp(*argv, "seq") == 0) {
+ if (seq_ok++)
+ duparg2("seq", *argv);
+ flags |= TUNNEL_SEQ;
} else {
break;
}
@@ -987,6 +1040,9 @@ static int parse_encap_ip6(struct rtattr *rta, size_t len,
argc--; argv++;
}
+ if (flags)
+ ret = rta_addattr16(rta, len, LWTUNNEL_IP6_FLAGS, flags);
+
/* argv is currently the first unparsed argument,
* but the lwt_parse_encap() caller will move to the next,
* so step back
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index 9603ac6..b9ae6e3 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -737,7 +737,8 @@ is a set of encapsulation attributes specific to the
.B tos
.IR TOS " ] ["
.B ttl
-.IR TTL " ]"
+.IR TTL " ] [ "
+.BR key " ] [" csum " ] [ " seq " ] "
.in -2
.sp
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH] net: usb: asix: ax88772_bind return error when hw_reset fail
From: Marcel Ziswiler @ 2019-01-24 14:03 UTC (permalink / raw)
To: davem@davemloft.net, zhang.run@zte.com.cn
Cc: wang.yi59@zte.com.cn, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, xue.zhihong@zte.com.cn,
lynxis@fe80.eu, linux-usb@vger.kernel.org
In-Reply-To: <1548308929-7727-1-git-send-email-zhang.run@zte.com.cn>
On Thu, 2019-01-24 at 13:48 +0800, Zhang Run wrote:
> The ax88772_bind() should return error code immediately when the PHY
> was not reset properly through ax88772a_hw_reset().
> Otherwise, The asix_get_phyid() will block when get the PHY
> Identifier from the PHYSID1 MII registers through asix_mdio_read()
> due to the PHY isn't ready. Furthermore, it will produce a lot of
> error message cause system crash.As follows:
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to write
> reg index 0x0000: -71
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to send
> software reset: ffffffb9
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to write
> reg index 0x0000: -71
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to enable
> software MII access
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to read
> reg index 0x0000: -71
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to write
> reg index 0x0000: -71
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to enable
> software MII access
> asix 1-1:1.0 (unnamed net_device) (uninitialized): Failed to read
> reg index 0x0000: -71
> ...
>
> Signed-off-by: Zhang Run <zhang.run@zte.com.cn>
> Reviewed-by: Yang Wei <yang.wei9@zte.com.cn>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
> ---
> drivers/net/usb/asix_devices.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/asix_devices.c
> b/drivers/net/usb/asix_devices.c
> index b654f05..3d93993 100644
> --- a/drivers/net/usb/asix_devices.c
> +++ b/drivers/net/usb/asix_devices.c
> @@ -739,8 +739,13 @@ static int ax88772_bind(struct usbnet *dev,
> struct usb_interface *intf)
> asix_read_cmd(dev, AX_CMD_STATMNGSTS_REG, 0, 0, 1, &chipcode,
> 0);
> chipcode &= AX_CHIPCODE_MASK;
>
> - (chipcode == AX_AX88772_CHIPCODE) ? ax88772_hw_reset(dev, 0) :
> - ax88772a_hw_reset(dev, 0);
> + ret = (chipcode == AX_AX88772_CHIPCODE) ? ax88772_hw_reset(dev,
> 0) :
> + ax88772a_hw_reset(dev
> , 0);
> +
> + if (ret < 0) {
> + netdev_dbg(dev->net, "Failed to reset AX88772: %d\n",
> ret);
> + return ret;
> + }
>
> /* Read PHYID register *AFTER* the PHY was reset properly */
> phyid = asix_get_phyid(dev);
^ permalink raw reply
* Re: [PATCH] sfp: sfp_read: split-up request when hw rx buffer is too small.
From: René van Dorst @ 2019-01-24 14:03 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <20190123213229.GA17772@lunn.ch>
Quoting Andrew Lunn <andrew@lunn.ch>:
> On Wed, Jan 23, 2019 at 10:20:46PM +0100, René van Dorst wrote:
>> Without this patch sfp code retries to read the full struct sfp_eeprom_id
>> id out of the SFP eeprom. Sizeof(id) is 96 bytes.
>> My i2c hardware, Mediatek mt7621, has a rx buffer of 64 bytes.
>> So sfp_read gets -NOSUPPORTED back on his turn return -EAGAIN.
>> Same issue is with the SFP_EXT_STATUS data which is 92 bytes.
>>
>> By split-up the request in multiple smaller requests with a max size of i2c
>> max_read_len, we can readout the SFP module successfully.
>>
>> Tested with MT7621 and two Fiberstore modules SFP-GB-GE-T and SFP-GE-BX.
>>
>> Signed-off-by: René van Dorst <opensource@vdorst.com>
>> ---
>> drivers/net/phy/sfp.c | 23 ++++++++++++++++++++++-
>> 1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
>> index fd8bb998ae52..1352a19571cd 100644
>> --- a/drivers/net/phy/sfp.c
>> +++ b/drivers/net/phy/sfp.c
>> @@ -367,7 +367,28 @@ static void sfp_set_state(struct sfp *sfp,
>> unsigned int state)
>>
>> static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf,
>> size_t len)
>> {
>> - return sfp->read(sfp, a2, addr, buf, len);
>> + const struct i2c_adapter_quirks *q = sfp->i2c->quirks;
>> + int ret;
>> + size_t rx_bytes = 0;
>> +
>> + /* Many i2c hw have limited rx buffers, split-up request when needed. */
>> + while ((q->max_read_len) && (len > q->max_read_len)) {
>> + ret = sfp->read(sfp, a2, addr, buf, q->max_read_len);
>
> Hi René
>
> I think you want to pass MIN(len, q->max_read_len) to read().
Hi Andrew,
max_read_len is 0 when there is no quirk.
I can write it a bit differently depending on the outcome of my other email.
>> + if (ret < 0)
>> + return ret;
>> + rx_bytes += ret;
>> + addr += q->max_read_len;
>> + buf += q->max_read_len;
>> + len -= q->max_read_len;
>
> I would prefer you add ret, not q->max_read_len. There is a danger it
> returned less bytes than you asked for.
Getting less bytes then asked is already an error I think.
I could check the return size and directly return the number of bytes that I
have. The callers are checking for size and they can retry if wanted. So that
should not be an issue.
>> + }
>> +
>> + ret = sfp->read(sfp, a2, addr, buf, len);
>> + if (ret < 0)
>> + return ret;
>> +
>> + rx_bytes += ret;
>> +
>> + return rx_bytes;
>> }
>>
>> static int sfp_write(struct sfp *sfp, bool a2, u8 addr, void *buf,
>> size_t len)
>
> Doesn't write need the same handling?
By reading the SSF spec we can write to a user writable EERPOM area of
120 bytes.
But the current code has only has 1 sfp_write for a byte value.
So for now I should say no.
Greats,
René
^ permalink raw reply
* Re: [PATCH v3] netfilter: nft_meta: Add NFT_META_I/OIFKIND meta type
From: Florian Westphal @ 2019-01-24 13:59 UTC (permalink / raw)
To: wenxu
Cc: Pablo Neira Ayuso, Florian Westphal, netdev, netfilter-devel,
David Ahern
In-Reply-To: <de38b79c-4e94-16bd-5389-9b0fc224f7eb@ucloud.cn>
wenxu <wenxu@ucloud.cn> wrote:
> Hi all,
> Are there any other idear for this patch? Maye should modify IFNAMSIZ to MODULE_NAME_LEN
i think its fine as-is.
^ permalink raw reply
* Re: [PATCH v3] netfilter: nft_meta: Add NFT_META_I/OIFKIND meta type
From: wenxu @ 2019-01-24 13:51 UTC (permalink / raw)
To: Pablo Neira Ayuso, Florian Westphal; +Cc: netdev, netfilter-devel, David Ahern
In-Reply-To: <20190118143503.3h2tv4fujl4w25mz@salvia>
Hi all,
Are there any other idear for this patch? Maye should modify IFNAMSIZ to MODULE_NAME_LEN
BR
wenxu
On 2019/1/18 下午10:35, Pablo Neira Ayuso wrote:
> On Fri, Jan 18, 2019 at 03:32:08PM +0100, Florian Westphal wrote:
>> Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>>>> + case NFT_META_IIFKIND:
>>>> + if (in == NULL || in->rtnl_link_ops == NULL)
>>>> + goto err;
>>>> + strncpy((char *)dest, in->rtnl_link_ops->kind, IFNAMSIZ);
>>> It seems kind can be arbitrarily large, no limitation in its length.
>> Its limited to 60 or 56 bytes it seems:
>> char kind[MODULE_NAME_LEN];
>>
>> nla_strlcpy(kind, linfo[IFLA_INFO_KIND], sizeof(kind));
>>
>> (linkinfo_to_kind_ops in rtnetlink.c).
>>
>>> Thinking...
>>>
>>> There is no other way to identify a vft device rather than this
>>> string? The only l3mdev that exists if vrf, right?
>> There is, I suggested this more generic approach, as it would allow
>> to create rules that match on the kind of device used (vrf, ppp, etc.).
> Ah I see.
>
>> If you think its too generic, ok.
> I think it's fine, thanks for explaining.
>
^ permalink raw reply
* [PATCH] net: macb: Apply RXUBR workaround only to versions with errata
From: Harini Katakam @ 2019-01-24 13:38 UTC (permalink / raw)
To: nicolas.ferre, davem, claudiu.beznea, brandon.streiff
Cc: netdev, linux-kernel, michal.simek, harinikatakamlinux,
harini.katakam
The interrupt handler contains a workaround for RX hang applicable
to Zynq and AT91 only. Subsequent versions do not need this
workaround. This workaround unecessarily resets RX whenever RX used
bit read is observed, which can be often under heavy traffic. Hence
introduce an CAPS mask and a check to enable this workaround.
Signed-off-by: Harini Katakam <harini.katakam@xilinx.com>
---
Changes from RFC:
- Use CAPS mask instead introducing and errata field.
- Use check only on RX reset part; ISR should still be cleared.
drivers/net/ethernet/cadence/macb.h | 1 +
drivers/net/ethernet/cadence/macb_main.c | 16 ++++++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/cadence/macb.h b/drivers/net/ethernet/cadence/macb.h
index 3d45f4c..2b412fa 100644
--- a/drivers/net/ethernet/cadence/macb.h
+++ b/drivers/net/ethernet/cadence/macb.h
@@ -643,6 +643,7 @@
#define MACB_CAPS_JUMBO 0x00000020
#define MACB_CAPS_GEM_HAS_PTP 0x00000040
#define MACB_CAPS_BD_RD_PREFETCH 0x00000080
+#define MACB_CAPS_NEEDS_RSTONUBR 0x00000100
#define MACB_CAPS_FIFO_MODE 0x10000000
#define MACB_CAPS_GIGABIT_MODE_AVAILABLE 0x20000000
#define MACB_CAPS_SG_DISABLED 0x40000000
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 66cc792..0bda1cd 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -1416,10 +1416,12 @@ static irqreturn_t macb_interrupt(int irq, void *dev_id)
* section 16.7.4 for details.
*/
if (status & MACB_BIT(RXUBR)) {
- ctrl = macb_readl(bp, NCR);
- macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
- wmb();
- macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+ if (bp->caps & MACB_CAPS_NEEDS_RSTONUBR) {
+ ctrl = macb_readl(bp, NCR);
+ macb_writel(bp, NCR, ctrl & ~MACB_BIT(RE));
+ wmb();
+ macb_writel(bp, NCR, ctrl | MACB_BIT(RE));
+ }
if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
queue_writel(queue, ISR, MACB_BIT(RXUBR));
@@ -3864,7 +3866,8 @@ static int at91ether_init(struct platform_device *pdev)
}
static const struct macb_config at91sam9260_config = {
- .caps = MACB_CAPS_USRIO_HAS_CLKEN | MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII,
+ .caps = MACB_CAPS_USRIO_HAS_CLKEN |
+ MACB_CAPS_USRIO_DEFAULT_IS_MII_GMII | MACB_CAPS_NEEDS_RSTONUBR,
.clk_init = macb_clk_init,
.init = macb_init,
};
@@ -3928,7 +3931,8 @@ static const struct macb_config zynqmp_config = {
};
static const struct macb_config zynq_config = {
- .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF,
+ .caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE | MACB_CAPS_NO_GIGABIT_HALF |
+ MACB_CAPS_NEEDS_RSTONUBR,
.dma_burst_length = 16,
.clk_init = macb_clk_init,
.init = macb_init,
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 02/10] net: macsec: convert to SPDX
From: Antoine Tenart @ 2019-01-24 13:29 UTC (permalink / raw)
To: Andrew Lunn
Cc: Antoine Tenart, davem, sd, f.fainelli, hkallweit1, netdev,
linux-kernel, thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
In-Reply-To: <20190124131118.GA28903@lunn.ch>
Hi Andrew,
On Thu, Jan 24, 2019 at 02:11:18PM +0100, Andrew Lunn wrote:
> > That's right, I'll fix it in v2. In such cases, the license text takes
> > precedence over the MODULE_LICENSE() definition ?
>
> I've also not seen any real discussion about this. But when i recently
> proposed patches to cleanup similar issues in drivers/net/phy, i gave
> the license text precedence, since it is a lot easier for a human to
> understand, and it is likely a human made the error....
I would go for this as well, it seems logical for MODULE_LICENSE() to
only be an identifier of what the license described in the license text
is.
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH net] net: mvpp2: fix condition for setting up link interrupt
From: Thomas Bogendoerfer @ 2019-01-24 13:18 UTC (permalink / raw)
To: David S. Miller, netdev, linux-kernel
Set up link interrupt if connection is handled by phylink otherwise
link state change detection for in-band-status doesn't work.
Fixes: 4bb043262878 ("net: mvpp2: phylink support")
Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 16066c2d5b3a..0fbfe1945a69 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -3405,7 +3405,7 @@ static int mvpp2_open(struct net_device *dev)
valid = true;
}
- if (priv->hw_version == MVPP22 && port->link_irq && !port->phylink) {
+ if (priv->hw_version == MVPP22 && port->link_irq && port->phylink) {
err = request_irq(port->link_irq, mvpp2_link_status_isr, 0,
dev->name, port);
if (err) {
--
2.16.4
^ permalink raw reply related
* Re: [PATCH bpf-next 3/3] xsk: add sock_diag interface for AF_XDP
From: Björn Töpel @ 2019-01-24 13:15 UTC (permalink / raw)
To: Daniel Borkmann
Cc: ast, Netdev, Björn Töpel, Karlsson, Magnus,
Magnus Karlsson
In-Reply-To: <959824cf-4e8c-9816-af78-a768b83bf3a0@iogearbox.net>
Den tors 24 jan. 2019 kl 14:08 skrev Daniel Borkmann <daniel@iogearbox.net>:
>
[...]
> >>> +#ifndef _LINUX_XDP_DIAG_H
> >>> +#define _LINUX_XDP_DIAG_H
> >>> +
> >>> +#include <linux/types.h>
> >>> +
> >>> +struct xdp_diag_req {
> >>> + __u8 sdiag_family;
> >>> + __u8 sdiag_protocol;
> >>> + __u16 pad;
> >>
> >> Presumably this one is for future use? Maybe better as '__u16 :16;' to
> >> avoid compile errors if someone tries to zero 'pad' member manually?
> >
> > The "pad" was there simply to have an explicitly named structure hole.
> > I'm not following the bitfield argument. Why does that avoid compiler
> > errors?
>
> Mostly in the sense that an application would explicitly set 'pad = 0'
> whereas pad could later on potentially be renamed and reused otherwise
> (which __u16 :16 would avoid in first place). But looking at other
> *_diag_req structs, it's explicitly named as 'pad' elsewhere already,
> then nevermind, lets have it rather consistent then and keep as is.
>
Ok, thanks for clearing it up. I'll address all your comments, and
also adds a "pad" member in msg for consistency.
> One small thing I still spotted when looking at it again, in function
> xsk_diag_handler_dump() the req is unused.
>
Ah, thank you.
Björn
> Thanks,
> Daniel
^ permalink raw reply
* Re: [PATCH] sfp: sfp_read: split-up request when hw rx buffer is too small.
From: René van Dorst @ 2019-01-24 13:15 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Andrew Lunn, Heiner Kallweit, David S. Miller, netdev
In-Reply-To: <c5ed6991-5b68-f514-2b12-d47d907cdff1@gmail.com>
Quoting Florian Fainelli <f.fainelli@gmail.com>:
> On 1/23/19 1:20 PM, René van Dorst wrote:
>> Without this patch sfp code retries to read the full struct sfp_eeprom_id
>> id out of the SFP eeprom. Sizeof(id) is 96 bytes.
>> My i2c hardware, Mediatek mt7621, has a rx buffer of 64 bytes.
>> So sfp_read gets -NOSUPPORTED back on his turn return -EAGAIN.
>> Same issue is with the SFP_EXT_STATUS data which is 92 bytes.
>>
>> By split-up the request in multiple smaller requests with a max size of i2c
>> max_read_len, we can readout the SFP module successfully.
>>
>> Tested with MT7621 and two Fiberstore modules SFP-GB-GE-T and SFP-GE-BX.
>>
>> Signed-off-by: René van Dorst <opensource@vdorst.com>
>> ---
>> drivers/net/phy/sfp.c | 23 ++++++++++++++++++++++-
>> 1 file changed, 22 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
>> index fd8bb998ae52..1352a19571cd 100644
>> --- a/drivers/net/phy/sfp.c
>> +++ b/drivers/net/phy/sfp.c
>> @@ -367,7 +367,28 @@ static void sfp_set_state(struct sfp *sfp,
>> unsigned int state)
>>
>> static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf,
>> size_t len)
>> {
>> - return sfp->read(sfp, a2, addr, buf, len);
>> + const struct i2c_adapter_quirks *q = sfp->i2c->quirks;
>> + int ret;
>> + size_t rx_bytes = 0;
>> +
>> + /* Many i2c hw have limited rx buffers, split-up request when needed. */
>> + while ((q->max_read_len) && (len > q->max_read_len)) {
>> + ret = sfp->read(sfp, a2, addr, buf, q->max_read_len);
>> + if (ret < 0)
>> + return ret;
>> + rx_bytes += ret;
>> + addr += q->max_read_len;
>> + buf += q->max_read_len;
>> + len -= q->max_read_len;
>> + }
>
> sfp->read defaults to sfp_i2c_read() but it is possible to override that
> by registering a custom sfp_bus instance (nothing does it today, but
> that could obviously change), so there is no guarantee that
> sfp->i2c->quirks is applicable unless sfp_i2c_read() is used.
>
> sfp_i2c_read() is presumably where the max_read_len splitting should
> occur, or better yet, should not i2c_transfer() take care of that on its
> own? That way there would be no layering violation of having to fetch
> the quirk bitmask for the i2c adapter being used, that is something that
> should belong in the core i2c framework.
Yes it is better to put it in sfp_i2c_read().
I think it is best to handle the split-up within the driver.
The driver knows how to talk to the device and may apply device quirks.
Also tda1004x [0] and TPM [1] driver also handles it within the driver itself.
TPM driver just try to send the want size and split-up request to
I2C_SMBUS_BLOCK_MAX when a -EOPNOTSUPP returns, just retries it a number of
times.
I can do the same but I have to pick a minimum size.
Looking in SSF-8472rev12.2.1 they don't limit the way you access the device.
So use I2C_SMBUS_BLOCK_MAX of 32 bytes is sufficient or lookup the i2c
quirk max_read_len is also an option.
Grepping thru the i2c busses I see only 2 devices which has less then 32 bytes
of buffer. i2c-nvidia-gpu (Nvidia GPU) and i2c-pmcmsp (microcontroller
MSP71xx).
Both are unlikely to be used for these kind of applications.
I think I2C_SMBUS_BLOCK_MAX is safe to use.
Greats,
René
[0]
https://elixir.bootlin.com/linux/v5.0-rc3/source/drivers/media/dvb-frontends/tda1004x.c#L319
[1]
https://elixir.bootlin.com/linux/v5.0-rc3/source/drivers/char/tpm/tpm_i2c_infineon.c#L158
^ permalink raw reply
* Re: eBPF equivalent of SKF_AD_HATYPE / skb->dev->type
From: Daniel Borkmann @ 2019-01-24 13:12 UTC (permalink / raw)
To: Maciej Żenczykowski; +Cc: Alexei Starovoitov, Linux NetDev
In-Reply-To: <CAHo-Oox2ZwqBgymSs=eLfdkMPRwaS5jc7iPV8ycUNp42ug2ifw@mail.gmail.com>
On 01/24/2019 12:59 PM, Maciej Żenczykowski wrote:
> Yeah, I'd like to be able to attach the same program to
> cellular/wifi/ethernet(usb) interface at ingress.
>
> I'll cook it up when I can, for now I'll just make do with 2 different
> ones to handle the with/without ethernet header cases.
Sounds good, thanks!
^ permalink raw reply
* Re: [PATCH net-next 02/10] net: macsec: convert to SPDX
From: Andrew Lunn @ 2019-01-24 13:11 UTC (permalink / raw)
To: Antoine Tenart
Cc: davem, sd, f.fainelli, hkallweit1, netdev, linux-kernel,
thomas.petazzoni, alexandre.belloni, quentin.schulz,
allan.nielsen
In-Reply-To: <20190124085456.GB3662@kwain>
> That's right, I'll fix it in v2. In such cases, the license text takes
> precedence over the MODULE_LICENSE() definition ?
Hi Antoine
IANAL
I've also not seen any real discussion about this. But when i recently
proposed patches to cleanup similar issues in drivers/net/phy, i gave
the license text precedence, since it is a lot easier for a human to
understand, and it is likely a human made the error....
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next 3/3] xsk: add sock_diag interface for AF_XDP
From: Daniel Borkmann @ 2019-01-24 13:08 UTC (permalink / raw)
To: Björn Töpel
Cc: ast, Netdev, Björn Töpel, Karlsson, Magnus,
Magnus Karlsson
In-Reply-To: <CAJ+HfNhuOegRbmi9=A50e_rxLttA7dDS_8GTQR6fneGgWmNsVw@mail.gmail.com>
On 01/23/2019 03:24 PM, Björn Töpel wrote:
> Den ons 23 jan. 2019 kl 14:19 skrev Daniel Borkmann <daniel@iogearbox.net>:
>>
>> On 01/18/2019 02:03 PM, bjorn.topel@gmail.com wrote:
>>> From: Björn Töpel <bjorn.topel@intel.com>
>>>
>>> This patch adds the sock_diag interface for querying sockets from user
>>> space. Tools like iproute2 ss(8) can use this interface to list open
>>> AF_XDP sockets.
>>>
>>> The user-space ABI is defined in linux/xdp_diag.h and includes netlink
>>> request and response structs. The request can query sockets and the
>>> response contains socket information about the rings, umems, inode and
>>> more.
>>>
>>> Signed-off-by: Björn Töpel <bjorn.topel@intel.com>
>>
>> Series looks good, few minor nits inline:
>>
>>> ---
>>> include/uapi/linux/xdp_diag.h | 72 +++++++++++++
>>> net/xdp/Kconfig | 8 ++
>>> net/xdp/Makefile | 1 +
>>> net/xdp/xsk.c | 6 +-
>>> net/xdp/xsk.h | 12 +++
>>> net/xdp/xsk_diag.c | 192 ++++++++++++++++++++++++++++++++++
>>> 6 files changed, 286 insertions(+), 5 deletions(-)
>>> create mode 100644 include/uapi/linux/xdp_diag.h
>>> create mode 100644 net/xdp/xsk.h
>>> create mode 100644 net/xdp/xsk_diag.c
>>>
>>> diff --git a/include/uapi/linux/xdp_diag.h b/include/uapi/linux/xdp_diag.h
>>> new file mode 100644
>>> index 000000000000..efe8ce281dce
>>> --- /dev/null
>>> +++ b/include/uapi/linux/xdp_diag.h
>>> @@ -0,0 +1,72 @@
>>> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
>>> +/*
>>> + * xdp_diag: interface for query/monitor XDP sockets
>>> + * Copyright(c) 2019 Intel Corporation.
>>> + */
>>> +
>>> +#ifndef _LINUX_XDP_DIAG_H
>>> +#define _LINUX_XDP_DIAG_H
>>> +
>>> +#include <linux/types.h>
>>> +
>>> +struct xdp_diag_req {
>>> + __u8 sdiag_family;
>>> + __u8 sdiag_protocol;
>>> + __u16 pad;
>>
>> Presumably this one is for future use? Maybe better as '__u16 :16;' to
>> avoid compile errors if someone tries to zero 'pad' member manually?
>
> The "pad" was there simply to have an explicitly named structure hole.
> I'm not following the bitfield argument. Why does that avoid compiler
> errors?
Mostly in the sense that an application would explicitly set 'pad = 0'
whereas pad could later on potentially be renamed and reused otherwise
(which __u16 :16 would avoid in first place). But looking at other
*_diag_req structs, it's explicitly named as 'pad' elsewhere already,
then nevermind, lets have it rather consistent then and keep as is.
One small thing I still spotted when looking at it again, in function
xsk_diag_handler_dump() the req is unused.
Thanks,
Daniel
^ permalink raw reply
* Re: UBSAN: Undefined behaviour in net/ipv4/ip_output.c
From: Kyungtae Kim @ 2019-01-24 13:03 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Byoungyoung Lee, DaeRyong Jeong, syzkaller, Network Development,
LKML
In-Reply-To: <CAF=yD-KcnV=89eE1Y2Cx4nXUrazHP7MsuirMZKAKfq1n62b=fw@mail.gmail.com>
That's my bad. Thank you for your comment and effort.
Regards,
Kyungtae Kim
On Wed, Jan 23, 2019 at 4:34 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Mon, Jan 21, 2019 at 2:25 PM Kyungtae Kim <kt0755@gmail.com> wrote:
> >
> > I'm reporting a bug in linux-5.0-rc2: "UBSAN: Undefined behaviour in
> > net/ipv4/ip_output.c"
> >
> > kernel config: https://kt0755.github.io/etc/config-5.0-rc2
> > repro: https://kt0755.github.io/etc/repro.b6a11.c
>
> Thanks for the report.
>
> That reproducer does not seem to exercise system call sendmsg. Did you
> mean to share another? Perhaps one that users SO_SNDBUFFORCE or
> sysctl_wmem_max.
>
> When constructing a reproducer, I did easily trigger an UBSAN warning
> when setting SO_SNDBUFFORCE to INT_MAX - 1, so I can imagine that this
> one can trigger, as well. As long as sk_sndbuf can be set to any
> integer value, we'll have to be careful with any such multiplication.
>
> > Integer overflow happened in __ip_append_data() when 2 * sk->sk_sndbuf
> > (at line 1004)
> > is larger than the boundary of the destination (i.e., int).
> > Some sanity check code right before it would help.
> >
> > =========================================
> > UBSAN: Undefined behaviour in net/ipv4/ip_output.c:1004:11
> > signed integer overflow:
> > 1282607372 * 2 cannot be represented in type 'int'
> > CPU: 0 PID: 8465 Comm: syz-executor4 Not tainted 5.0.0-rc2 #1
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0xb1/0x118 lib/dump_stack.c:113
> > ubsan_epilogue+0x12/0x94 lib/ubsan.c:159
> > handle_overflow+0x1cf/0x21a lib/ubsan.c:190
> > __ubsan_handle_mul_overflow+0x2a/0x35 lib/ubsan.c:214
> > __ip_append_data+0x30ed/0x3350 net/ipv4/ip_output.c:1004
> > ip_append_data.part.18+0xf3/0x170 net/ipv4/ip_output.c:1220
> > ip_append_data+0x63/0x80 net/ipv4/ip_output.c:1209
> > raw_sendmsg+0xd99/0x2b60 net/ipv4/raw.c:670
> > inet_sendmsg+0xfc/0x620 net/ipv4/af_inet.c:798
> > sock_sendmsg_nosec net/socket.c:621 [inline]
> > sock_sendmsg+0xdd/0x130 net/socket.c:631
> > ___sys_sendmsg+0x7b3/0x950 net/socket.c:2116
> > __sys_sendmsg+0xfc/0x1d0 net/socket.c:2154
> > __do_sys_sendmsg net/socket.c:2163 [inline]
> > __se_sys_sendmsg net/socket.c:2161 [inline]
> > __x64_sys_sendmsg+0x83/0xc0 net/socket.c:2161
> > do_syscall_64+0xbe/0x4f0 arch/x86/entry/common.c:290
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > RIP: 0033:0x4497b9
> > Code: e8 8c 9f 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48
> > 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
> > 01 f0 ff ff 0f 83 9b 6b fc ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007f0d0100bc68 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > RAX: ffffffffffffffda RBX: 00007f0d0100c6cc RCX: 00000000004497b9
> > RDX: 0000000000000000 RSI: 0000000020000500 RDI: 0000000000000013
> > RBP: 000000000071bea0 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
> > R13: 00000000000089a0 R14: 00000000006f1a40 R15: 00007f0d0100c700
> > =========================================
> >
> > Thanks,
> > Kyungtae
^ permalink raw reply
* RE: [PATCH 1/3] treewide: Lift switch variables out of switches
From: Edwin Zimmerman @ 2019-01-24 12:58 UTC (permalink / raw)
To: 'Kees Cook', linux-kernel
Cc: 'Ard Biesheuvel', 'Laura Abbott',
'Alexander Popov', xen-devel, dri-devel, intel-gfx,
intel-wired-lan, netdev, linux-usb, linux-fsdevel, linux-mm, dev,
linux-kbuild, linux-security-module, kernel-hardening
In-Reply-To: <20190123110349.35882-2-keescook@chromium.org>
On Wednesday, January 23, 2019 6:04 AM, Kees Cook wrote
>
> Variables declared in a switch statement before any case statements
> cannot be initialized, so move all instances out of the switches.
> After this, future always-initialized stack variables will work
> and not throw warnings like this:
>
> fs/fcntl.c: In function ‘send_sigio_to_task’:
> fs/fcntl.c:738:13: warning: statement will never be executed [-Wswitch-unreachable]
> siginfo_t si;
> ^~
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Reviewed by: Edwin Zimmerman <edwin@211mainstreet.net>
> ---
> arch/x86/xen/enlighten_pv.c | 7 ++++---
> drivers/char/pcmcia/cm4000_cs.c | 2 +-
> drivers/char/ppdev.c | 20 ++++++++-----------
> drivers/gpu/drm/drm_edid.c | 4 ++--
> drivers/gpu/drm/i915/intel_display.c | 2 +-
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> drivers/net/ethernet/intel/e1000/e1000_main.c | 3 ++-
> drivers/tty/n_tty.c | 3 +--
> drivers/usb/gadget/udc/net2280.c | 5 ++---
> fs/fcntl.c | 3 ++-
> mm/shmem.c | 5 +++--
> net/core/skbuff.c | 4 ++--
> net/ipv6/ip6_gre.c | 4 ++--
> net/ipv6/ip6_tunnel.c | 4 ++--
> net/openvswitch/flow_netlink.c | 7 +++----
> security/tomoyo/common.c | 3 ++-
> security/tomoyo/condition.c | 7 ++++---
> security/tomoyo/util.c | 4 ++--
> 18 files changed, 45 insertions(+), 46 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index c54a493e139a..a79d4b548a08 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -907,14 +907,15 @@ static u64 xen_read_msr_safe(unsigned int msr, int *err)
> static int xen_write_msr_safe(unsigned int msr, unsigned low, unsigned high)
> {
> int ret;
> +#ifdef CONFIG_X86_64
> + unsigned which;
> + u64 base;
> +#endif
>
> ret = 0;
>
> switch (msr) {
> #ifdef CONFIG_X86_64
> - unsigned which;
> - u64 base;
> -
> case MSR_FS_BASE: which = SEGBASE_FS; goto set;
> case MSR_KERNEL_GS_BASE: which = SEGBASE_GS_USER; goto set;
> case MSR_GS_BASE: which = SEGBASE_GS_KERNEL; goto set;
> diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
> index 7a4eb86aedac..7211dc0e6f4f 100644
> --- a/drivers/char/pcmcia/cm4000_cs.c
> +++ b/drivers/char/pcmcia/cm4000_cs.c
> @@ -663,6 +663,7 @@ static void monitor_card(struct timer_list *t)
> {
> struct cm4000_dev *dev = from_timer(dev, t, timer);
> unsigned int iobase = dev->p_dev->resource[0]->start;
> + unsigned char flags0;
> unsigned short s;
> struct ptsreq ptsreq;
> int i, atrc;
> @@ -731,7 +732,6 @@ static void monitor_card(struct timer_list *t)
> }
>
> switch (dev->mstate) {
> - unsigned char flags0;
> case M_CARDOFF:
> DEBUGP(4, dev, "M_CARDOFF\n");
> flags0 = inb(REG_FLAGS0(iobase));
> diff --git a/drivers/char/ppdev.c b/drivers/char/ppdev.c
> index 1ae77b41050a..d77c97e4f996 100644
> --- a/drivers/char/ppdev.c
> +++ b/drivers/char/ppdev.c
> @@ -359,14 +359,19 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
> struct pp_struct *pp = file->private_data;
> struct parport *port;
> void __user *argp = (void __user *)arg;
> + struct ieee1284_info *info;
> + unsigned char reg;
> + unsigned char mask;
> + int mode;
> + s32 time32[2];
> + s64 time64[2];
> + struct timespec64 ts;
> + int ret;
>
> /* First handle the cases that don't take arguments. */
> switch (cmd) {
> case PPCLAIM:
> {
> - struct ieee1284_info *info;
> - int ret;
> -
> if (pp->flags & PP_CLAIMED) {
> dev_dbg(&pp->pdev->dev, "you've already got it!\n");
> return -EINVAL;
> @@ -517,15 +522,6 @@ static int pp_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
>
> port = pp->pdev->port;
> switch (cmd) {
> - struct ieee1284_info *info;
> - unsigned char reg;
> - unsigned char mask;
> - int mode;
> - s32 time32[2];
> - s64 time64[2];
> - struct timespec64 ts;
> - int ret;
> -
> case PPRSTATUS:
> reg = parport_read_status(port);
> if (copy_to_user(argp, ®, sizeof(reg)))
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b506e3622b08..8f93956c1628 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3942,12 +3942,12 @@ static void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid)
> }
>
> for_each_cea_db(cea, i, start, end) {
> + int sad_count;
> +
> db = &cea[i];
> dbl = cea_db_payload_len(db);
>
> switch (cea_db_tag(db)) {
> - int sad_count;
> -
> case AUDIO_BLOCK:
> /* Audio Data Block, contains SADs */
> sad_count = min(dbl / 3, 15 - total_sad_count);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3da9c0f9e948..aa1c2ebea456 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11341,6 +11341,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> drm_for_each_connector_iter(connector, &conn_iter) {
> struct drm_connector_state *connector_state;
> struct intel_encoder *encoder;
> + unsigned int port_mask;
>
> connector_state = drm_atomic_get_new_connector_state(state, connector);
> if (!connector_state)
> @@ -11354,7 +11355,6 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> WARN_ON(!connector_state->crtc);
>
> switch (encoder->type) {
> - unsigned int port_mask;
> case INTEL_OUTPUT_DDI:
> if (WARN_ON(!HAS_DDI(to_i915(dev))))
> break;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a26b4eddda25..c135fdec96b3 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -478,9 +478,9 @@ static void vlv_get_fifo_size(struct intel_crtc_state *crtc_state)
> struct vlv_fifo_state *fifo_state = &crtc_state->wm.vlv.fifo_state;
> enum pipe pipe = crtc->pipe;
> int sprite0_start, sprite1_start;
> + uint32_t dsparb, dsparb2, dsparb3;
>
> switch (pipe) {
> - uint32_t dsparb, dsparb2, dsparb3;
> case PIPE_A:
> dsparb = I915_READ(DSPARB);
> dsparb2 = I915_READ(DSPARB2);
> @@ -1944,6 +1944,7 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> const struct vlv_fifo_state *fifo_state =
> &crtc_state->wm.vlv.fifo_state;
> int sprite0_start, sprite1_start, fifo_size;
> + uint32_t dsparb, dsparb2, dsparb3;
>
> if (!crtc_state->fifo_changed)
> return;
> @@ -1969,7 +1970,6 @@ static void vlv_atomic_update_fifo(struct intel_atomic_state *state,
> spin_lock(&dev_priv->uncore.lock);
>
> switch (crtc->pipe) {
> - uint32_t dsparb, dsparb2, dsparb3;
> case PIPE_A:
> dsparb = I915_READ_FW(DSPARB);
> dsparb2 = I915_READ_FW(DSPARB2);
> diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
> index 8fe9af0e2ab7..041062736845 100644
> --- a/drivers/net/ethernet/intel/e1000/e1000_main.c
> +++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
> @@ -3140,8 +3140,9 @@ static netdev_tx_t e1000_xmit_frame(struct sk_buff *skb,
>
> hdr_len = skb_transport_offset(skb) + tcp_hdrlen(skb);
> if (skb->data_len && hdr_len == len) {
> + unsigned int pull_size;
> +
> switch (hw->mac_type) {
> - unsigned int pull_size;
> case e1000_82544:
> /* Make sure we have room to chop off 4 bytes,
> * and that the end alignment will work out to
> diff --git a/drivers/tty/n_tty.c b/drivers/tty/n_tty.c
> index 5dc9686697cf..eafb39157281 100644
> --- a/drivers/tty/n_tty.c
> +++ b/drivers/tty/n_tty.c
> @@ -634,6 +634,7 @@ static size_t __process_echoes(struct tty_struct *tty)
> while (MASK(ldata->echo_commit) != MASK(tail)) {
> c = echo_buf(ldata, tail);
> if (c == ECHO_OP_START) {
> + unsigned int num_chars, num_bs;
> unsigned char op;
> int no_space_left = 0;
>
> @@ -652,8 +653,6 @@ static size_t __process_echoes(struct tty_struct *tty)
> op = echo_buf(ldata, tail + 1);
>
> switch (op) {
> - unsigned int num_chars, num_bs;
> -
> case ECHO_OP_ERASE_TAB:
> if (MASK(ldata->echo_commit) == MASK(tail + 2))
> goto not_yet_stored;
> diff --git a/drivers/usb/gadget/udc/net2280.c b/drivers/usb/gadget/udc/net2280.c
> index e7dae5379e04..2b275a574e94 100644
> --- a/drivers/usb/gadget/udc/net2280.c
> +++ b/drivers/usb/gadget/udc/net2280.c
> @@ -2854,16 +2854,15 @@ static void ep_clear_seqnum(struct net2280_ep *ep)
> static void handle_stat0_irqs_superspeed(struct net2280 *dev,
> struct net2280_ep *ep, struct usb_ctrlrequest r)
> {
> + struct net2280_ep *e;
> int tmp = 0;
> + u16 status;
>
> #define w_value le16_to_cpu(r.wValue)
> #define w_index le16_to_cpu(r.wIndex)
> #define w_length le16_to_cpu(r.wLength)
>
> switch (r.bRequest) {
> - struct net2280_ep *e;
> - u16 status;
> -
> case USB_REQ_SET_CONFIGURATION:
> dev->addressed_state = !w_value;
> goto usb3_delegate;
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 083185174c6d..0640b64ecdc2 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -725,6 +725,8 @@ static void send_sigio_to_task(struct task_struct *p,
> struct fown_struct *fown,
> int fd, int reason, enum pid_type type)
> {
> + kernel_siginfo_t si;
> +
> /*
> * F_SETSIG can change ->signum lockless in parallel, make
> * sure we read it once and use the same value throughout.
> @@ -735,7 +737,6 @@ static void send_sigio_to_task(struct task_struct *p,
> return;
>
> switch (signum) {
> - kernel_siginfo_t si;
> default:
> /* Queue a rt signal with the appropriate fd as its
> value. We use SI_SIGIO as the source, not
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 6ece1e2fe76e..0b02624dd8b2 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1721,6 +1721,9 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
> swap_free(swap);
>
> } else {
> + loff_t i_size;
> + pgoff_t off;
> +
> if (vma && userfaultfd_missing(vma)) {
> *fault_type = handle_userfault(vmf, VM_UFFD_MISSING);
> return 0;
> @@ -1734,8 +1737,6 @@ static int shmem_getpage_gfp(struct inode *inode, pgoff_t index,
> if (shmem_huge == SHMEM_HUGE_FORCE)
> goto alloc_huge;
> switch (sbinfo->huge) {
> - loff_t i_size;
> - pgoff_t off;
> case SHMEM_HUGE_NEVER:
> goto alloc_nohuge;
> case SHMEM_HUGE_WITHIN_SIZE:
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 26d848484912..7597b3fc9d21 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4506,9 +4506,9 @@ static __sum16 *skb_checksum_setup_ip(struct sk_buff *skb,
> typeof(IPPROTO_IP) proto,
> unsigned int off)
> {
> - switch (proto) {
> - int err;
> + int err;
>
> + switch (proto) {
> case IPPROTO_TCP:
> err = skb_maybe_pull_tail(skb, off + sizeof(struct tcphdr),
> off + MAX_TCP_HDR_LEN);
> diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c
> index b1be67ca6768..9aee1add46c0 100644
> --- a/net/ipv6/ip6_gre.c
> +++ b/net/ipv6/ip6_gre.c
> @@ -427,9 +427,11 @@ static int ip6gre_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> u8 type, u8 code, int offset, __be32 info)
> {
> struct net *net = dev_net(skb->dev);
> + struct ipv6_tlv_tnl_enc_lim *tel;
> const struct ipv6hdr *ipv6h;
> struct tnl_ptk_info tpi;
> struct ip6_tnl *t;
> + __u32 teli;
>
> if (gre_parse_header(skb, &tpi, NULL, htons(ETH_P_IPV6),
> offset) < 0)
> @@ -442,8 +444,6 @@ static int ip6gre_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
> return -ENOENT;
>
> switch (type) {
> - struct ipv6_tlv_tnl_enc_lim *tel;
> - __u32 teli;
> case ICMPV6_DEST_UNREACH:
> net_dbg_ratelimited("%s: Path to destination invalid or inactive!\n",
> t->parms.name);
> diff --git a/net/ipv6/ip6_tunnel.c b/net/ipv6/ip6_tunnel.c
> index 0c6403cf8b52..94ccc7a9037b 100644
> --- a/net/ipv6/ip6_tunnel.c
> +++ b/net/ipv6/ip6_tunnel.c
> @@ -478,10 +478,12 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
> struct net *net = dev_net(skb->dev);
> u8 rel_type = ICMPV6_DEST_UNREACH;
> u8 rel_code = ICMPV6_ADDR_UNREACH;
> + struct ipv6_tlv_tnl_enc_lim *tel;
> __u32 rel_info = 0;
> struct ip6_tnl *t;
> int err = -ENOENT;
> int rel_msg = 0;
> + __u32 mtu, teli;
> u8 tproto;
> __u16 len;
>
> @@ -501,8 +503,6 @@ ip6_tnl_err(struct sk_buff *skb, __u8 ipproto, struct inet6_skb_parm *opt,
> err = 0;
>
> switch (*type) {
> - struct ipv6_tlv_tnl_enc_lim *tel;
> - __u32 mtu, teli;
> case ICMPV6_DEST_UNREACH:
> net_dbg_ratelimited("%s: Path to destination invalid or inactive!\n",
> t->parms.name);
> diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
> index 691da853bef5..dee2f9516ae8 100644
> --- a/net/openvswitch/flow_netlink.c
> +++ b/net/openvswitch/flow_netlink.c
> @@ -2652,8 +2652,11 @@ static int validate_set(const struct nlattr *a,
> u8 mac_proto, __be16 eth_type, bool masked, bool log)
> {
> const struct nlattr *ovs_key = nla_data(a);
> + const struct ovs_key_ipv4 *ipv4_key;
> + const struct ovs_key_ipv6 *ipv6_key;
> int key_type = nla_type(ovs_key);
> size_t key_len;
> + int err;
>
> /* There can be only one key in a action */
> if (nla_total_size(nla_len(ovs_key)) != nla_len(a))
> @@ -2671,10 +2674,6 @@ static int validate_set(const struct nlattr *a,
> return -EINVAL;
>
> switch (key_type) {
> - const struct ovs_key_ipv4 *ipv4_key;
> - const struct ovs_key_ipv6 *ipv6_key;
> - int err;
> -
> case OVS_KEY_ATTR_PRIORITY:
> case OVS_KEY_ATTR_SKB_MARK:
> case OVS_KEY_ATTR_CT_MARK:
> diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
> index c598aa00d5e3..bedbd0518153 100644
> --- a/security/tomoyo/common.c
> +++ b/security/tomoyo/common.c
> @@ -1583,8 +1583,9 @@ static void tomoyo_read_domain(struct tomoyo_io_buffer *head)
> list_for_each_cookie(head->r.domain, &tomoyo_domain_list) {
> struct tomoyo_domain_info *domain =
> list_entry(head->r.domain, typeof(*domain), list);
> + u8 i;
> +
> switch (head->r.step) {
> - u8 i;
> case 0:
> if (domain->is_deleted &&
> !head->r.print_this_domain_only)
> diff --git a/security/tomoyo/condition.c b/security/tomoyo/condition.c
> index 8d0e1b9c9c57..c10d903febe5 100644
> --- a/security/tomoyo/condition.c
> +++ b/security/tomoyo/condition.c
> @@ -787,10 +787,11 @@ bool tomoyo_condition(struct tomoyo_request_info *r,
> /* Check string expressions. */
> if (right == TOMOYO_NAME_UNION) {
> const struct tomoyo_name_union *ptr = names_p++;
> + struct tomoyo_path_info *symlink;
> + struct tomoyo_execve *ee;
> + struct file *file;
> +
> switch (left) {
> - struct tomoyo_path_info *symlink;
> - struct tomoyo_execve *ee;
> - struct file *file;
> case TOMOYO_SYMLINK_TARGET:
> symlink = obj ? obj->symlink_target : NULL;
> if (!symlink ||
> diff --git a/security/tomoyo/util.c b/security/tomoyo/util.c
> index badffc8271c8..8e2bb36df37b 100644
> --- a/security/tomoyo/util.c
> +++ b/security/tomoyo/util.c
> @@ -668,6 +668,8 @@ static bool tomoyo_file_matches_pattern2(const char *filename,
> {
> while (filename < filename_end && pattern < pattern_end) {
> char c;
> + int i, j;
> +
> if (*pattern != '\\') {
> if (*filename++ != *pattern++)
> return false;
> @@ -676,8 +678,6 @@ static bool tomoyo_file_matches_pattern2(const char *filename,
> c = *filename;
> pattern++;
> switch (*pattern) {
> - int i;
> - int j;
> case '?':
> if (c == '/') {
> return false;
> --
> 2.17.1
^ permalink raw reply
* Re: [PATCH] ipvs: Fix signed integer overflow when setsockopt timeout
From: Pablo Neira Ayuso @ 2019-01-24 12:32 UTC (permalink / raw)
To: ZhangXiaoxu; +Cc: wensong, horms, ja, netdev, lvs-devel, netfilter-devel
In-Reply-To: <1547109546-11344-1-git-send-email-zhangxiaoxu5@huawei.com>
On Thu, Jan 10, 2019 at 04:39:06PM +0800, ZhangXiaoxu wrote:
> There is a UBSAN bug report as below:
> UBSAN: Undefined behaviour in net/netfilter/ipvs/ip_vs_ctl.c:2227:21
> signed integer overflow:
> -2147483647 * 1000 cannot be represented in type 'int'
Applied, thanks.
^ permalink raw reply
* Re: [PATCH mlx5-next 0/4] mlx5 next misc updates
From: Leon Romanovsky @ 2019-01-24 12:30 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
Jason Gunthorpe
In-Reply-To: <20190119003313.16711-1-saeedm@mellanox.com>
[-- Attachment #1: Type: text/plain, Size: 853 bytes --]
On Fri, Jan 18, 2019 at 04:33:09PM -0800, Saeed Mahameed wrote:
> Hi all,
>
> This series includes updates to mlx5-next shared branch.
>
> 1) from Jason, improve mlx5_cmd_exec_cb async API to be safer
> 2) from Maxim Mikityanskiy, cleanups for mlx5_write64 doorbell API
> 3) from Michael Guralnik, Add pci AtomicOps request
>
> Thanks,
> Saeed.
>
> ---
>
> Jason Gunthorpe (1):
> net/mlx5: Make mlx5_cmd_exec_cb() a safe API
>
> Michael Guralnik (1):
> net/mlx5: Add pci AtomicOps request
Those two were applied to mlx5-next branch.
ce4eee5340a9 (mlx5-next) net/mlx5: Add pci AtomicOps request
e355477ed9e4 net/mlx5: Make mlx5_cmd_exec_cb() a safe API
> Maxim Mikityanskiy (2):
> net/mlx5: Remove unused MLX5_*_DOORBELL_LOCK macros
> net/mlx5: Remove spinlock support from mlx5_write64
Those two needs extra work,
Thanks
>
> --
> 2.20.1
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: Question: pause mode disabled for marvell 88e151x phy
From: Yunsheng Lin @ 2019-01-24 12:28 UTC (permalink / raw)
To: Russell King - ARM Linux admin
Cc: Andrew Lunn, Florian Fainelli, davem@davemloft.net,
netdev@vger.kernel.org, Weiwei Deng, Yisen.Zhuang@huawei.com,
huangdaode@hisilicon.com, lipeng321@huawei.com,
salil.mehta@huawei.com, lijianhua 00216010,
linux-kernel@vger.kernel.org
In-Reply-To: <20190123224144.imzntelxqtf3xcuj@e5254000004ec.dyn.armlinux.org.uk>
On 2019/1/24 6:41, Russell King - ARM Linux admin wrote:
> On Sat, Jan 05, 2019 at 11:28:19AM +0800, Yunsheng Lin wrote:
>> On 2018/12/17 22:36, Russell King - ARM Linux wrote:
>>> I'll try to do further diagnosis over Christmas in case I've missed
>>> something, but I suspect it may be one of those "weird behaviour" issues
>>> where the safest action is to disable pause mode as per my commit -
>>> which is far saner than having mismatched pause status on either end
>>> of a link. However, given that Marvell specs are all NDA-only, it's
>>> very difficult to investigate beyond "this is the observed behaviour".
>>
>> Hi,
>>
>> Is there any update on the further diagnosis?
>
> Hi,
>
> I've finally been able to do some further diagnosis (with a 'scope).
> It would appear that the network adapter had PHY polling enabled,
> which meant that it overwrote the PHYs advertisement register
> during negotiation. I thought I'd checked that scenario, but alas
> clearing the PHY poll enable bit on its own doesn't stop it polling!
>
> I'll send a revert for the commit shortly.
Great. Thanks for the further diagnosis.
>
^ permalink raw reply
* Re: [PATCH rdma-next 00/12] Add SRQ and XRC support for ODP MRs
From: Leon Romanovsky @ 2019-01-24 12:25 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: RDMA mailing list, Majd Dibbiny, Moni Shoua, Saeed Mahameed,
linux-netdev
In-Reply-To: <20190122064851.6032-1-leon@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1193 bytes --]
On Tue, Jan 22, 2019 at 08:48:39AM +0200, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Hi,
>
> This series extend ODP to work with SRQ and XRC. Being both per-operation
> (e.g. RDMA write, RDMA read and atomic) and per-transport (e.g. RC, UD and XRC),
> we extend IB/core and mlx5 driver to provide needed information to user space.
>
> Thanks
>
> Moni Shoua (12):
> IB/mlx5: Fix locking SRQ object in ODP event
> IB/core: Allocate bit for SRQ ODP support
> IB/uverbs: Expose XRC ODP device capabilities
> IB/mlx5: Remove useless check in ODP handler
> IB/mlx5: Clean mlx5_ib_mr_responder_pfault_handler() signature
> IB/mlx5: Add XRC initiator ODP support
> IB/mlx5: Let read user wqe also from SRQ buffer
> IB/mlx5: Add ODP SRQ support
> IB/mlx5: Advertise SRQ ODP support for supported transports
> net/mlx5: Add XRC transport to ODP device capabilities layout
> IB/mlx5: Advertise XRC ODP support
> net/mlx5: Set ODP SRQ support in firmware
Doug, Jason,
Please pay attention that patches below should go to shared branch:
net/mlx5: Add XRC transport to ODP device capabilities layout
net/mlx5: Set ODP SRQ support in firmware
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ 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