* Re: [PATCH] DECnet: Use container_of() for embedded struct
From: David Miller @ 2017-05-09 13:40 UTC (permalink / raw)
To: keescook; +Cc: linux-kernel, linux-decnet-user, netdev
In-Reply-To: <20170508223144.GA53216@beast>
From: Kees Cook <keescook@chromium.org>
Date: Mon, 8 May 2017 15:31:44 -0700
> Instead of a direct cross-type cast, use conatiner_of() to locate
> the embedded structure, even in the face of future struct layout
> randomization.
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Applied.
^ permalink raw reply
* Re: [PATCH net] ipv6: make sure dev is not NULL before call ip6_frag_reasm
From: Hangbin Liu @ 2017-05-09 13:40 UTC (permalink / raw)
To: Eric Dumazet; +Cc: netdev
In-Reply-To: <1494255030.7796.56.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, May 08, 2017 at 07:50:30AM -0700, Eric Dumazet wrote:
> On Mon, 2017-05-08 at 11:09 +0800, Hangbin Liu wrote:
> > Since ip6_frag_reasm() will call __in6_dev_get(dev), which will access
> > dev->ip6_ptr. We need to make sure dev is not NULL.
> >
> > Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> > ---
> > net/ipv6/reassembly.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> > index e1da5b8..e3ebd62 100644
> > --- a/net/ipv6/reassembly.c
> > +++ b/net/ipv6/reassembly.c
> > @@ -348,7 +348,7 @@ static int ip6_frag_queue(struct frag_queue *fq, struct sk_buff *skb,
> > fq->q.flags |= INET_FRAG_FIRST_IN;
> > }
> >
> > - if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> > + if (dev && fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) &&
> > fq->q.meat == fq->q.len) {
> > int res;
> > unsigned long orefdst = skb->_skb_refdst;
>
>
> How dev could be possibly NULL here ?
I saw we checked the dev in this function
dev = skb->dev;
if (dev) {
fq->iif = dev->ifindex;
skb->dev = NULL;
}
and upper caller ipv6_frag_rcv()
fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
Apologise that I did not do enough research to make sure whether skb->dev
could be NULL or not. I will do the check recently and reply when got a
confirmation.
Regards
Hangbin
^ permalink raw reply
* Re: [PATCH] drivers: net: wimax: i2400m: i2400m-usb: Use time_after for time comparison
From: David Miller @ 2017-05-09 13:40 UTC (permalink / raw)
To: karim.eshapa; +Cc: inaky.perez-gonzalez, linux-wimax, netdev, linux-kernel
In-Reply-To: <1494288410-19028-1-git-send-email-karim.eshapa@gmail.com>
From: Karim Eshapa <karim.eshapa@gmail.com>
Date: Tue, 9 May 2017 02:06:50 +0200
> Use time_after() for time comparison with the new fix.
>
> Signed-off-by: Karim Eshapa <karim.eshapa@gmail.com>
This looks a lot better, applied, thanks.
^ permalink raw reply
* [PATCH] net: qca_spi: Fix alignment issues in rx path
From: Stefan Wahren @ 2017-05-09 13:40 UTC (permalink / raw)
To: David S. Miller; +Cc: Greg Kroah-Hartman, netdev, linux-kernel, Stefan Wahren
The qca_spi driver causes alignment issues on ARM devices.
So fix this by using netdev_alloc_skb_ip_align().
Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Fixes: 291ab06ecf67 ("net: qualcomm: new Ethernet over SPI driver for QCA7000")
---
drivers/net/ethernet/qualcomm/qca_spi.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/qualcomm/qca_spi.c b/drivers/net/ethernet/qualcomm/qca_spi.c
index 513e6c7..24ca7df 100644
--- a/drivers/net/ethernet/qualcomm/qca_spi.c
+++ b/drivers/net/ethernet/qualcomm/qca_spi.c
@@ -296,8 +296,9 @@ qcaspi_receive(struct qcaspi *qca)
/* Allocate rx SKB if we don't have one available. */
if (!qca->rx_skb) {
- qca->rx_skb = netdev_alloc_skb(net_dev,
- net_dev->mtu + VLAN_ETH_HLEN);
+ qca->rx_skb = netdev_alloc_skb_ip_align(net_dev,
+ net_dev->mtu +
+ VLAN_ETH_HLEN);
if (!qca->rx_skb) {
netdev_dbg(net_dev, "out of RX resources\n");
qca->stats.out_of_mem++;
@@ -377,7 +378,7 @@ qcaspi_receive(struct qcaspi *qca)
qca->rx_skb, qca->rx_skb->dev);
qca->rx_skb->ip_summed = CHECKSUM_UNNECESSARY;
netif_rx_ni(qca->rx_skb);
- qca->rx_skb = netdev_alloc_skb(net_dev,
+ qca->rx_skb = netdev_alloc_skb_ip_align(net_dev,
net_dev->mtu + VLAN_ETH_HLEN);
if (!qca->rx_skb) {
netdev_dbg(net_dev, "out of RX resources\n");
@@ -759,7 +760,8 @@ qcaspi_netdev_init(struct net_device *dev)
if (!qca->rx_buffer)
return -ENOBUFS;
- qca->rx_skb = netdev_alloc_skb(dev, qca->net_dev->mtu + VLAN_ETH_HLEN);
+ qca->rx_skb = netdev_alloc_skb_ip_align(dev, qca->net_dev->mtu +
+ VLAN_ETH_HLEN);
if (!qca->rx_skb) {
kfree(qca->rx_buffer);
netdev_info(qca->net_dev, "Failed to allocate RX sk_buff.\n");
--
2.1.4
^ permalink raw reply related
* [PATCH v7 0/5] skb_to_sgvec hardening
From: Jason A. Donenfeld @ 2017-05-09 13:50 UTC (permalink / raw)
To: netdev, linux-kernel, davem, kernel-hardening; +Cc: Jason A. Donenfeld
The recent bug with macsec and historical one with virtio have
indicated that letting skb_to_sgvec trounce all over an sglist
without checking the length is probably a bad idea. And it's not
necessary either: an sglist already explicitly marks its last
item, and the initialization functions are diligent in doing so.
Thus there's a clear way of avoiding future overflows.
So, this patchset, from a high level, makes skb_to_sgvec return
a potential error code, and then adjusts all callers to check
for the error code. There are two situations in which skb_to_sgvec
might return such an error:
1) When the passed in sglist is too small; and
2) When the passed in skbuff is too deeply nested.
So, the first patch in this series handles the issues with
skb_to_sgvec directly, and the remaining ones then handle the call
sites.
Jason A. Donenfeld (5):
skbuff: return -EMSGSIZE in skb_to_sgvec to prevent overflow
ipsec: check return value of skb_to_sgvec always
rxrpc: check return value of skb_to_sgvec always
macsec: check return value of skb_to_sgvec always
virtio_net: check return value of skb_to_sgvec always
drivers/net/macsec.c | 13 ++++++++--
drivers/net/virtio_net.c | 4 ++-
net/core/skbuff.c | 65 +++++++++++++++++++++++++++++++-----------------
net/ipv4/ah4.c | 8 ++++--
net/ipv4/esp4.c | 30 ++++++++++++++--------
net/ipv6/ah6.c | 8 ++++--
net/ipv6/esp6.c | 31 +++++++++++++++--------
net/rxrpc/rxkad.c | 13 +++++++---
8 files changed, 119 insertions(+), 53 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH v7 1/5] skbuff: return -EMSGSIZE in skb_to_sgvec to prevent overflow
From: Jason A. Donenfeld @ 2017-05-09 13:50 UTC (permalink / raw)
To: netdev, linux-kernel, davem, kernel-hardening
Cc: Jason A. Donenfeld, Steffen Klassert, Herbert Xu, David Howells,
Sabrina Dubroca, Michael S. Tsirkin, Jason Wang
In-Reply-To: <20170509135009.13751-1-Jason@zx2c4.com>
This is a defense-in-depth measure in response to bugs like
4d6fa57b4dab ("macsec: avoid heap overflow in skb_to_sgvec"). There's
not only a potential overflow of sglist items, but also a stack overflow
potential, so we fix this by limiting the amount of recursion this function
is allowed to do. Not actually providing a bounded base case is a future
disaster that we can easily avoid here.
As a small matter of house keeping, we take this opportunity to move the
documentation comment over the actual function the documentation is for.
While this could be implemented by using an explicit stack of skbuffs,
when implementing this, the function complexity increased considerably,
and I don't think such complexity and bloat is actually worth it. So,
instead I built this and tested it on x86, x86_64, ARM, ARM64, and MIPS,
and measured the stack usage there. I also reverted the recent MIPS
changes that give it a separate IRQ stack, so that I could experience
some worst-case situations. I found that limiting it to 24 layers deep
yielded a good stack usage with room for safety, as well as being much
deeper than any driver actually ever creates.
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: David Howells <dhowells@redhat.com>
Cc: Sabrina Dubroca <sd@queasysnail.net>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
net/core/skbuff.c | 65 +++++++++++++++++++++++++++++++++++--------------------
1 file changed, 42 insertions(+), 23 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index f86bf69cfb8d..ab51b97d5600 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3481,24 +3481,18 @@ void __init skb_init(void)
NULL);
}
-/**
- * skb_to_sgvec - Fill a scatter-gather list from a socket buffer
- * @skb: Socket buffer containing the buffers to be mapped
- * @sg: The scatter-gather list to map into
- * @offset: The offset into the buffer's contents to start mapping
- * @len: Length of buffer space to be mapped
- *
- * Fill the specified scatter-gather list with mappings/pointers into a
- * region of the buffer space attached to a socket buffer.
- */
static int
-__skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
+__skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len,
+ unsigned int recursion_level)
{
int start = skb_headlen(skb);
int i, copy = start - offset;
struct sk_buff *frag_iter;
int elt = 0;
+ if (unlikely(recursion_level >= 24))
+ return -EMSGSIZE;
+
if (copy > 0) {
if (copy > len)
copy = len;
@@ -3517,6 +3511,8 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
end = start + skb_frag_size(&skb_shinfo(skb)->frags[i]);
if ((copy = end - offset) > 0) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
+ if (unlikely(elt && sg_is_last(&sg[elt - 1])))
+ return -EMSGSIZE;
if (copy > len)
copy = len;
@@ -3531,16 +3527,22 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
}
skb_walk_frags(skb, frag_iter) {
- int end;
+ int end, ret;
WARN_ON(start > offset + len);
end = start + frag_iter->len;
if ((copy = end - offset) > 0) {
+ if (unlikely(elt && sg_is_last(&sg[elt - 1])))
+ return -EMSGSIZE;
+
if (copy > len)
copy = len;
- elt += __skb_to_sgvec(frag_iter, sg+elt, offset - start,
- copy);
+ ret = __skb_to_sgvec(frag_iter, sg+elt, offset - start,
+ copy, recursion_level + 1);
+ if (unlikely(ret < 0))
+ return ret;
+ elt += ret;
if ((len -= copy) == 0)
return elt;
offset += copy;
@@ -3551,6 +3553,31 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
return elt;
}
+/**
+ * skb_to_sgvec - Fill a scatter-gather list from a socket buffer
+ * @skb: Socket buffer containing the buffers to be mapped
+ * @sg: The scatter-gather list to map into
+ * @offset: The offset into the buffer's contents to start mapping
+ * @len: Length of buffer space to be mapped
+ *
+ * Fill the specified scatter-gather list with mappings/pointers into a
+ * region of the buffer space attached to a socket buffer. Returns either
+ * the number of scatterlist items used, or -EMSGSIZE if the contents
+ * could not fit.
+ */
+int skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
+{
+ int nsg = __skb_to_sgvec(skb, sg, offset, len, 0);
+
+ if (nsg <= 0)
+ return nsg;
+
+ sg_mark_end(&sg[nsg - 1]);
+
+ return nsg;
+}
+EXPORT_SYMBOL_GPL(skb_to_sgvec);
+
/* As compared with skb_to_sgvec, skb_to_sgvec_nomark only map skb to given
* sglist without mark the sg which contain last skb data as the end.
* So the caller can mannipulate sg list as will when padding new data after
@@ -3573,19 +3600,11 @@ __skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
int skb_to_sgvec_nomark(struct sk_buff *skb, struct scatterlist *sg,
int offset, int len)
{
- return __skb_to_sgvec(skb, sg, offset, len);
+ return __skb_to_sgvec(skb, sg, offset, len, 0);
}
EXPORT_SYMBOL_GPL(skb_to_sgvec_nomark);
-int skb_to_sgvec(struct sk_buff *skb, struct scatterlist *sg, int offset, int len)
-{
- int nsg = __skb_to_sgvec(skb, sg, offset, len);
- sg_mark_end(&sg[nsg - 1]);
-
- return nsg;
-}
-EXPORT_SYMBOL_GPL(skb_to_sgvec);
/**
* skb_cow_data - Check that a socket buffer's data buffers are writable
--
2.12.2
^ permalink raw reply related
* [PATCH v7 2/5] ipsec: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 13:50 UTC (permalink / raw)
To: netdev, linux-kernel, davem, kernel-hardening
Cc: Jason A. Donenfeld, Steffen Klassert, Herbert Xu
In-Reply-To: <20170509135009.13751-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Steffen Klassert <steffen.klassert@secunet.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: "David S. Miller" <davem@davemloft.net>
---
net/ipv4/ah4.c | 8 ++++++--
net/ipv4/esp4.c | 30 ++++++++++++++++++++----------
net/ipv6/ah6.c | 8 ++++++--
net/ipv6/esp6.c | 31 +++++++++++++++++++++----------
4 files changed, 53 insertions(+), 24 deletions(-)
diff --git a/net/ipv4/ah4.c b/net/ipv4/ah4.c
index 22377c8ff14b..e8f862358518 100644
--- a/net/ipv4/ah4.c
+++ b/net/ipv4/ah4.c
@@ -220,7 +220,9 @@ static int ah_output(struct xfrm_state *x, struct sk_buff *skb)
ah->seq_no = htonl(XFRM_SKB_CB(skb)->seq.output.low);
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
@@ -393,7 +395,9 @@ static int ah_input(struct xfrm_state *x, struct sk_buff *skb)
skb_push(skb, ihl);
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index b1e24446e297..42cb09cc8533 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -360,9 +360,13 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_extra(skb, esph, extra);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0)) {
+ spin_unlock_bh(&x->lock);
+ goto error;
+ }
allocsize = ALIGN(skb->data_len, L1_CACHE_BYTES);
@@ -381,11 +385,13 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
pfrag->offset = pfrag->offset + allocsize;
sg_init_table(dsg, skb_shinfo(skb)->nr_frags + 1);
- skb_to_sgvec(skb, dsg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, dsg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
spin_unlock_bh(&x->lock);
+ if (unlikely(err < 0))
+ goto error;
goto skip_cow2;
}
@@ -422,9 +428,11 @@ static int esp_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_extra(skb, esph, extra);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0))
+ goto error;
skip_cow2:
if ((x->props.flags & XFRM_STATE_ESN))
@@ -658,7 +666,9 @@ static int esp_input(struct xfrm_state *x, struct sk_buff *skb)
esp_input_set_header(skb, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ err = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out;
skb->ip_summed = CHECKSUM_NONE;
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index dda6035e3b84..755f38271dd5 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -423,7 +423,9 @@ static int ah6_output(struct xfrm_state *x, struct sk_buff *skb)
ah->seq_no = htonl(XFRM_SKB_CB(skb)->seq.output.low);
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
@@ -606,7 +608,9 @@ static int ah6_input(struct xfrm_state *x, struct sk_buff *skb)
ip6h->hop_limit = 0;
sg_init_table(sg, nfrags + sglists);
- skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ err = skb_to_sgvec_nomark(skb, sg, 0, skb->len);
+ if (unlikely(err < 0))
+ goto out_free;
if (x->props.flags & XFRM_STATE_ESN) {
/* Attach seqhi sg right after packet payload */
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index ff54faa75631..017e2c2d36e1 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -339,9 +339,13 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_esn(skb, esph, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0)) {
+ spin_unlock_bh(&x->lock);
+ goto error;
+ }
allocsize = ALIGN(skb->data_len, L1_CACHE_BYTES);
@@ -360,12 +364,15 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
pfrag->offset = pfrag->offset + allocsize;
sg_init_table(dsg, skb_shinfo(skb)->nr_frags + 1);
- skb_to_sgvec(skb, dsg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, dsg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
spin_unlock_bh(&x->lock);
+ if (unlikely(err < 0))
+ goto error;
+
goto skip_cow2;
}
}
@@ -403,9 +410,11 @@ static int esp6_output(struct xfrm_state *x, struct sk_buff *skb)
esph = esp_output_set_esn(skb, esph, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg,
- (unsigned char *)esph - skb->data,
- assoclen + ivlen + clen + alen);
+ err = skb_to_sgvec(skb, sg,
+ (unsigned char *)esph - skb->data,
+ assoclen + ivlen + clen + alen);
+ if (unlikely(err < 0))
+ goto error;
skip_cow2:
if ((x->props.flags & XFRM_STATE_ESN))
@@ -600,7 +609,9 @@ static int esp6_input(struct xfrm_state *x, struct sk_buff *skb)
esp_input_set_header(skb, seqhi);
sg_init_table(sg, nfrags);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ ret = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(ret < 0))
+ goto out;
skb->ip_summed = CHECKSUM_NONE;
--
2.12.2
^ permalink raw reply related
* [PATCH v7 3/5] rxrpc: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 13:50 UTC (permalink / raw)
To: netdev, linux-kernel, davem, kernel-hardening
Cc: Jason A. Donenfeld, David Howells
In-Reply-To: <20170509135009.13751-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: David Howells <dhowells@redhat.com>
---
net/rxrpc/rxkad.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/net/rxrpc/rxkad.c b/net/rxrpc/rxkad.c
index 4374e7b9c7bf..486026689448 100644
--- a/net/rxrpc/rxkad.c
+++ b/net/rxrpc/rxkad.c
@@ -229,7 +229,9 @@ static int rxkad_secure_packet_encrypt(const struct rxrpc_call *call,
len &= ~(call->conn->size_align - 1);
sg_init_table(sg, nsg);
- skb_to_sgvec(skb, sg, 0, len);
+ err = skb_to_sgvec(skb, sg, 0, len);
+ if (unlikely(err < 0))
+ goto out;
skcipher_request_set_crypt(req, sg, sg, len, iv.x);
crypto_skcipher_encrypt(req);
@@ -342,7 +344,8 @@ static int rxkad_verify_packet_1(struct rxrpc_call *call, struct sk_buff *skb,
goto nomem;
sg_init_table(sg, nsg);
- skb_to_sgvec(skb, sg, offset, 8);
+ if (unlikely(skb_to_sgvec(skb, sg, offset, 8) < 0))
+ goto nomem;
/* start the decryption afresh */
memset(&iv, 0, sizeof(iv));
@@ -429,7 +432,11 @@ static int rxkad_verify_packet_2(struct rxrpc_call *call, struct sk_buff *skb,
}
sg_init_table(sg, nsg);
- skb_to_sgvec(skb, sg, offset, len);
+ if (unlikely(skb_to_sgvec(skb, sg, offset, len) < 0)) {
+ if (sg != _sg)
+ kfree(sg);
+ goto nomem;
+ }
/* decrypt from the session key */
token = call->conn->params.key->payload.data[0];
--
2.12.2
^ permalink raw reply related
* [PATCH v7 4/5] macsec: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 13:50 UTC (permalink / raw)
To: netdev, linux-kernel, davem, kernel-hardening
Cc: Jason A. Donenfeld, Sabrina Dubroca
In-Reply-To: <20170509135009.13751-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Sabrina Dubroca <sd@queasysnail.net>
---
drivers/net/macsec.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 49ce4e9f4a0f..e022e3fcd012 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -742,7 +742,12 @@ static struct sk_buff *macsec_encrypt(struct sk_buff *skb,
macsec_fill_iv(iv, secy->sci, pn);
sg_init_table(sg, ret);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ ret = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(ret < 0)) {
+ macsec_txsa_put(tx_sa);
+ kfree_skb(skb);
+ return ERR_PTR(ret);
+ }
if (tx_sc->encrypt) {
int len = skb->len - macsec_hdr_len(sci_present) -
@@ -952,7 +957,11 @@ static struct sk_buff *macsec_decrypt(struct sk_buff *skb,
macsec_fill_iv(iv, sci, ntohl(hdr->packet_number));
sg_init_table(sg, ret);
- skb_to_sgvec(skb, sg, 0, skb->len);
+ ret = skb_to_sgvec(skb, sg, 0, skb->len);
+ if (unlikely(ret < 0)) {
+ kfree_skb(skb);
+ return ERR_PTR(ret);
+ }
if (hdr->tci_an & MACSEC_TCI_E) {
/* confidentiality: ethernet + macsec header
--
2.12.2
^ permalink raw reply related
* [PATCH v7 5/5] virtio_net: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 13:50 UTC (permalink / raw)
To: netdev, linux-kernel, davem, kernel-hardening
Cc: Jason A. Donenfeld, Michael S. Tsirkin, Jason Wang
In-Reply-To: <20170509135009.13751-1-Jason@zx2c4.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
---
drivers/net/virtio_net.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index f36584616e7d..1709fd0b4bf7 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1081,7 +1081,7 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
struct virtio_net_hdr_mrg_rxbuf *hdr;
const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
struct virtnet_info *vi = sq->vq->vdev->priv;
- unsigned num_sg;
+ int num_sg;
unsigned hdr_len = vi->hdr_len;
bool can_push;
@@ -1114,6 +1114,8 @@ static int xmit_skb(struct send_queue *sq, struct sk_buff *skb)
sg_set_buf(sq->sg, hdr, hdr_len);
num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
}
+ if (unlikely(num_sg < 0))
+ return num_sg;
return virtqueue_add_outbuf(sq->vq, sq->sg, num_sg, skb, GFP_ATOMIC);
}
--
2.12.2
^ permalink raw reply related
* Re: Marvell phy errata origins?
From: Daniel Walker @ 2017-05-09 13:58 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Andy Fleming, Harini Katakam,
netdev@vger.kernel.org, HEMANT RAMDASI,
Julius Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
In-Reply-To: <20170418140450.GB13724@lunn.ch>
On 04/18/2017 07:04 AM, Andrew Lunn wrote:
> On Tue, Apr 18, 2017 at 06:16:33AM -0700, Daniel Walker wrote:
>> Hi,
>>
>> Cisco is using a Marvell 88E1112 phy. It seems to be fairly similar
>> to the 88E1111 which Harini added a fix for.
> Hi Daniel
>
> If you look at Marvell reference drive, DSDT, they are actually quite
> different. Different virtual cable tester, different downshift
> configuration, different packet generator, different loopback. I would
> say they are different generations of PHY.
>
>> In Harini's commit
>> message for ,
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/net/phy/marvell.c?id=3ec0a0f10ceb
>>
>> "This function has a sequence accessing Page 5 and Register 31, both
>> of which are not defined or reserved for this PHY"
>>
>> For the 88E1112 we see that these are "Factory Test Modes" which the
>> contents of are not documented. They aren't really "not defied", and
>> aren't really "reserved" .. Marvell support claims they don't
>> support these drivers, and Freescale seems to be adding these
>> drivers, and the line we are looking at.
>>
>> We had some issues with our PHY which were corrected with the same
>> patch Harini used but modified for the M88E1112. We're trying to get
>> to the bottom of where this code came from and what it was suppose
>> to be doing.
> I tried to find this errata fix in the Marvell reference code. And
> failed to find it. But it is "Vendor Crap" code, hard to find anything
> in it.
>
> My guess is, this errata just applies to one model of PHY, maybe even
> one revision of one model of a PHY. The hard bit is figuring out what
> actually needs it. Do you have access to Marvell datasheets?
According to Marvell this was errata for 88M1101 , and should not be
applied to any other phy .. So we should be removing these lines and
make a special aneg for 88M1101 then restore everything that doesn't
need this back to the generic aneg,
Daniel
^ permalink raw reply
* Re: [PATCH v7 0/5] skb_to_sgvec hardening
From: Johannes Berg @ 2017-05-09 14:03 UTC (permalink / raw)
To: Jason A. Donenfeld, netdev, linux-kernel, davem, kernel-hardening
In-Reply-To: <20170509135009.13751-1-Jason@zx2c4.com>
On Tue, 2017-05-09 at 15:50 +0200, Jason A. Donenfeld wrote:
> The recent bug with macsec and historical one with virtio have
> indicated that letting skb_to_sgvec trounce all over an sglist
> without checking the length is probably a bad idea. And it's not
> necessary either: an sglist already explicitly marks its last
> item, and the initialization functions are diligent in doing so.
> Thus there's a clear way of avoiding future overflows.
>
> So, this patchset, from a high level, makes skb_to_sgvec return
> a potential error code, and then adjusts all callers to check
> for the error code.
Perhaps you should add __must_check annotation to the function
prototype(s)?
johannes
^ permalink raw reply
* Re: [PATCH v7 0/5] skb_to_sgvec hardening
From: Jason A. Donenfeld @ 2017-05-09 14:08 UTC (permalink / raw)
To: Johannes Berg; +Cc: Netdev, LKML, David Miller, kernel-hardening
In-Reply-To: <1494338600.2410.6.camel@sipsolutions.net>
On Tue, May 9, 2017 at 4:03 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> Perhaps you should add __must_check annotation to the function
> prototype(s)?
Great idea. I've started doing this in my own code. Wasn't sure how
popular it was outside of there, but I'm glad to hear a suggestion of
it now. I'll have this in v8.
^ permalink raw reply
* Re: [PATCH v7 2/5] ipsec: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 14:09 UTC (permalink / raw)
To: Netdev, LKML, David Miller, kernel-hardening
Cc: Jason A. Donenfeld, Steffen Klassert, Herbert Xu
In-Reply-To: <20170509135009.13751-3-Jason@zx2c4.com>
(The next submission of this ipsec patch will have this rebased over
the latest upstream tree.
https://git.zx2c4.com/linux-dev/log/?h=jd/safe-skb-vec )
^ permalink raw reply
* Re: [PATCH v7 5/5] virtio_net: check return value of skb_to_sgvec always
From: Jason A. Donenfeld @ 2017-05-09 14:10 UTC (permalink / raw)
To: Netdev, LKML, David Miller, kernel-hardening
Cc: Jason A. Donenfeld, Michael S. Tsirkin, Jason Wang
In-Reply-To: <20170509135009.13751-6-Jason@zx2c4.com>
On Tue, May 9, 2017 at 3:50 PM, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> num_sg = skb_to_sgvec(skb, sq->sg + 1, 0, skb->len) + 1;
(The next submission of this will take into account this + 1 here.
https://git.zx2c4.com/linux-dev/log/?h=jd/safe-skb-vec )
^ permalink raw reply
* Re: [PATCH 0/4] TI Bluetooth serdev support
From: Rob Herring @ 2017-05-09 14:14 UTC (permalink / raw)
To: Baruch Siach
Cc: Adam Ford, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Johan Hedberg,
Gustavo Padovan, Marcel Holtmann, Sebastian Reichel, Wei Xu,
open list:BLUETOOTH DRIVERS, Eyal Reizer, netdev, Satish Patel,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20170509044837.oje2tfodytyuuuur-MwjkAAnuF3khR1HGirfZ1z4kX+cae0hd@public.gmane.org>
On Mon, May 8, 2017 at 11:48 PM, Baruch Siach <baruch-NswTu9S1W3P6gbPvEgmw2w@public.gmane.org> wrote:
> Hi Rob,
>
> On Mon, May 08, 2017 at 04:12:16PM -0500, Rob Herring wrote:
>> On Fri, May 5, 2017 at 9:51 AM, Adam Ford <aford173-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> > On Sun, Apr 30, 2017 at 11:04 AM, Sebastian Reichel <sre-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >> On Sun, Apr 30, 2017 at 10:14:20AM -0500, Adam Ford wrote:
>> >>> On Wed, Apr 5, 2017 at 1:30 PM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> >>> > This series adds serdev support to the HCI LL protocol used on TI BT
>> >>> > modules and enables support on HiKey board with with the WL1835 module.
>> >>> > With this the custom TI UIM daemon and btattach are no longer needed.
>> >>>
>> >>> Without UIM daemon, what instruction do you use to load the BT firmware?
>> >>>
>> >>> I was thinking 'hciattach' but I was having trouble. I was hoping you
>> >>> might have some insight.
>> >>>
>> >>> hciattach -t 30 -s 115200 /dev/ttymxc1 texas 3000000 flow Just
>> >>> returns a timeout.
>> >>>
>> >>> I modified my i.MX6 device tree per the binding documentation and
>> >>> setup the regulators and enable GPIO pins.
>> >>
>> >> If you configured everything correctly no userspace interaction is
>> >> required. The driver should request the firmware automatically once
>> >> you power up the bluetooth device.
>> >>
>> >> Apart from DT changes make sure, that the following options are
>> >> enabled and check dmesg for any hints.
>> >>
>> >> CONFIG_SERIAL_DEV_BUS
>> >> CONFIG_SERIAL_DEV_CTRL_TTYPORT
>> >> CONFIG_BT_HCIUART
>> >> CONFIG_BT_HCIUART_LL
>> >
>> > I have enabled those flags, and I have updated my device tree.
>> > I am testing this on an OMAP3630 (DM3730) board with a WL1283. I am
>> > getting a lot of timeout errors. I tested this against the original
>> > implemention I had in pdata-quirks.c using the ti-st driver, uim & and
>> > the btwilink driver.
>> >
>> > I pulled in some of the newer patches to enable the wl1283-st, but I
>> > am obviously missing something.
>> >
>> > I 58.717651] Bluetooth: hci0: Reading TI version information failed
>> > (-110)
>> > [ 58.724853] Bluetooth: hci0: download firmware failed, retrying...
>> > [ 60.957641] Bluetooth: hci0 command 0x1001 tx timeout
>> > [ 68.957641] Bluetooth: hci0: Reading TI version information failed
>> > (-110)
>> > [ 68.964843] Bluetooth: hci0: download firmware failed, retrying...
>> > [ 69.132171] Bluetooth: Unknown HCI packet type 06
>> > [ 69.138244] Bluetooth: Unknown HCI packet type 0c
>> > [ 69.143249] Bluetooth: Unknown HCI packet type 40
>> > [ 69.148498] Bluetooth: Unknown HCI packet type 20
>> > [ 69.153533] Bluetooth: Data length is too large
>> > [ 69.158569] Bluetooth: Unknown HCI packet type a0
>> > [ 69.163574] Bluetooth: Unknown HCI packet type 00
>> > [ 69.168731] Bluetooth: Unknown HCI packet type 00
>> > [ 69.173736] Bluetooth: Unknown HCI packet type 34
>> > [ 69.178924] Bluetooth: Unknown HCI packet type 91
>> > [ 71.197631] Bluetooth: hci0 command 0x1001 tx timeout
>> > [ 79.197662] Bluetooth: hci0: Reading TI version information failed (-110)
>>
>> There's a bug in serdev_device_write(), so if you have that function
>> you need either the fix I sent or the patch to make
>> serdev_device_writebuf atomic again. Both are on the linux-serial
>> list, but not in any tree yet.
>
> You refer to the patches below, right?
>
> [PATCH] tty: serdev: fix serdev_device_write return value,
> http://www.spinics.net/lists/linux-serial/msg26117.html
>
> [PATCH] serdev: Restore serdev_device_write_buf for atomic context,
> http://www.spinics.net/lists/linux-serial/msg26113.html
Yes, either one will fix the issue.
Rob
^ permalink raw reply
* [PATCH 0/4] hamradio: Fine-tuning for nine function implementations
From: SF Markus Elfring @ 2017-05-09 14:21 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 16:11:23 +0200
A few update suggestions were taken into account
from static source code analysis.
Markus Elfring (4):
Combine two seq_printf() calls into one in yam_seq_show()
Adjust four function calls together with a variable assignment
Use seq_puts() in bpq_seq_show()
Adjust four function calls together with a variable assignment
drivers/net/hamradio/bpqether.c | 14 +++++++++-----
drivers/net/hamradio/yam.c | 15 +++++++++------
2 files changed, 18 insertions(+), 11 deletions(-)
--
2.12.2
^ permalink raw reply
* [PATCH 1/4] hamradio: Combine two seq_printf() calls into one in yam_seq_show()
From: SF Markus Elfring @ 2017-05-09 14:22 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 14:16:45 +0200
A bit of data was put into a sequence by two separate function calls.
Print the same data by a single function call instead.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/yam.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
index b6891ada1d7b..542f1e511df1 100644
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -830,8 +830,7 @@ static int yam_seq_show(struct seq_file *seq, void *v)
seq_printf(seq, " RxFrames %lu\n", dev->stats.rx_packets);
seq_printf(seq, " TxInt %u\n", yp->nb_mdint);
seq_printf(seq, " RxInt %u\n", yp->nb_rxint);
- seq_printf(seq, " RxOver %lu\n", dev->stats.rx_fifo_errors);
- seq_printf(seq, "\n");
+ seq_printf(seq, " RxOver %lu\n\n", dev->stats.rx_fifo_errors);
return 0;
}
--
2.12.2
^ permalink raw reply related
* [PATCH 2/4] hamradio: Adjust four function calls together with a variable assignment
From: SF Markus Elfring @ 2017-05-09 14:23 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 15:15:16 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix affected source code places.
Improve a size determination.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/yam.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hamradio/yam.c b/drivers/net/hamradio/yam.c
index 542f1e511df1..c792b0f116a5 100644
--- a/drivers/net/hamradio/yam.c
+++ b/drivers/net/hamradio/yam.c
@@ -401,7 +401,8 @@ static unsigned char *add_mcs(unsigned char *bits, int bitrate,
}
/* Allocate a new mcs */
- if ((p = kmalloc(sizeof(struct yam_mcs), GFP_KERNEL)) == NULL) {
+ p = kmalloc(sizeof(*p), GFP_KERNEL);
+ if (!p) {
release_firmware(fw);
return NULL;
}
@@ -549,7 +550,8 @@ static inline void yam_rx_flag(struct net_device *dev, struct yam_port *yp)
if ((yp->rx_crch & yp->rx_crcl) != 0xFF) {
/* Bad crc */
} else {
- if (!(skb = dev_alloc_skb(pkt_len))) {
+ skb = dev_alloc_skb(pkt_len);
+ if (!skb) {
printk(KERN_WARNING "%s: memory squeeze, dropping packet\n", dev->name);
++dev->stats.rx_dropped;
} else {
@@ -670,7 +672,8 @@ static void yam_tx_byte(struct net_device *dev, struct yam_port *yp)
break;
case TX_HEAD:
if (--yp->tx_count <= 0) {
- if (!(skb = skb_dequeue(&yp->send_queue))) {
+ skb = skb_dequeue(&yp->send_queue);
+ if (!skb) {
ptt_off(dev);
yp->tx_state = TX_OFF;
break;
@@ -879,7 +882,8 @@ static int yam_open(struct net_device *dev)
printk(KERN_ERR "%s: cannot 0x%lx busy\n", dev->name, dev->base_addr);
return -EACCES;
}
- if ((u = yam_check_uart(dev->base_addr)) == c_uart_unknown) {
+ u = yam_check_uart(dev->base_addr);
+ if (u == c_uart_unknown) {
printk(KERN_ERR "%s: cannot find uart type\n", dev->name);
ret = -EIO;
goto out_release_base;
--
2.12.2
^ permalink raw reply related
* [PATCH 3/4] hamradio: Use seq_puts() in bpq_seq_show()
From: SF Markus Elfring @ 2017-05-09 14:24 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 15:45:09 +0200
A string which did not contain a data format specification should be put
into a sequence. Thus use the corresponding function "seq_puts".
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/bpqether.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index f62e7f325cf9..eaa0f2e8e561 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -434,7 +434,7 @@ static int bpq_seq_show(struct seq_file *seq, void *v)
bpqdev->dest_addr);
if (is_multicast_ether_addr(bpqdev->acpt_addr))
- seq_printf(seq, "*\n");
+ seq_puts(seq, "*\n");
else
seq_printf(seq, "%pM\n", bpqdev->acpt_addr);
--
2.12.2
^ permalink raw reply related
* Re: Marvell phy errata origins?
From: Andrew Lunn @ 2017-05-09 14:24 UTC (permalink / raw)
To: Daniel Walker
Cc: Florian Fainelli, Andy Fleming, Harini Katakam,
netdev@vger.kernel.org, HEMANT RAMDASI,
Julius Hemanth Pitti -X (jpitti - MONTA VISTA SOFTWARE INC at Cisco)
In-Reply-To: <ffd12ac5-7501-0310-182f-1735c2da8165@cisco.com>
> According to Marvell this was errata for 88M1101 , and should not be
> applied to any other phy .. So we should be removing these lines and
> make a special aneg for 88M1101 then restore everything that doesn't
> need this back to the generic aneg,
Hi Daniel
Thanks for finding this out. Can you role a patch?
Andrew
^ permalink raw reply
* [PATCH 4/4] hamradio: Adjust four function calls together with a variable assignment
From: SF Markus Elfring @ 2017-05-09 14:25 UTC (permalink / raw)
To: linux-hams, netdev, David S. Miller, Javier Martinez Canillas,
Jean-Paul Roubelat
Cc: LKML, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 9 May 2017 15:57:17 +0200
The script "checkpatch.pl" pointed information out like the following.
ERROR: do not use assignment in if condition
Thus fix affected source code places.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/net/hamradio/bpqether.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/hamradio/bpqether.c b/drivers/net/hamradio/bpqether.c
index eaa0f2e8e561..5e234e0ca256 100644
--- a/drivers/net/hamradio/bpqether.c
+++ b/drivers/net/hamradio/bpqether.c
@@ -185,7 +185,8 @@ static int bpq_rcv(struct sk_buff *skb, struct net_device *dev, struct packet_ty
if (!net_eq(dev_net(dev), &init_net))
goto drop;
- if ((skb = skb_share_check(skb, GFP_ATOMIC)) == NULL)
+ skb = skb_share_check(skb, GFP_ATOMIC);
+ if (!skb)
return NET_RX_DROP;
if (!pskb_may_pull(skb, sizeof(struct ethhdr)))
@@ -286,7 +287,8 @@ static netdev_tx_t bpq_xmit(struct sk_buff *skb, struct net_device *dev)
bpq = netdev_priv(dev);
orig_dev = dev;
- if ((dev = bpq_get_ether_dev(dev)) == NULL) {
+ dev = bpq_get_ether_dev(dev);
+ if (!dev) {
orig_dev->stats.tx_dropped++;
kfree_skb(skb);
return NETDEV_TX_OK;
@@ -565,12 +567,14 @@ static int bpq_device_event(struct notifier_block *this,
break;
case NETDEV_DOWN: /* ethernet device closed -> close BPQ interface */
- if ((dev = bpq_get_ax25_dev(dev)) != NULL)
+ dev = bpq_get_ax25_dev(dev);
+ if (dev)
dev_close(dev);
break;
case NETDEV_UNREGISTER: /* ethernet device removed -> free BPQ interface */
- if ((dev = bpq_get_ax25_dev(dev)) != NULL)
+ dev = bpq_get_ax25_dev(dev);
+ if (dev)
bpq_free_device(dev);
break;
default:
--
2.12.2
^ permalink raw reply related
* RE: sky2: Use seq_putc() in sky2_debug_show()
From: David Laight @ 2017-05-09 14:26 UTC (permalink / raw)
To: 'Stephen Hemminger', SF Markus Elfring
Cc: Lino Sanfilippo, netdev@vger.kernel.org, Mirko Lindner, LKML,
kernel-janitors@vger.kernel.org
In-Reply-To: <20170508225012.508f24c0@xeon-e3>
From: Stephen Hemminger
> Sent: 09 May 2017 06:50
> On Mon, 8 May 2017 19:42:46 +0200
> SF Markus Elfring <elfring@users.sourceforge.net> wrote:
>
> > > Which issue do you mean? I dont see any issue you fix here.
> >
> > Are the run time characteristics a bit nicer for the function seq_putc
> > in comparison to the function seq_puts for printing a single line break here?
> >
> > Regards,
> > Markus
>
> I would put this in why bother category. seq_puts is correct and this is only
> in diagnostic output useful to developer and disabled on most distro kernels
Sometimes consistency is best.
Output everything with seq_printf(), using a format "%s" if necessary.
The performance really doesn't matter here at all.
It is also (probably) possible to get gcc to do the conversions - as it does for printf().
(A right PITA for embedded systems where only printf() exists.)
David
^ permalink raw reply
* Re: [PATCH 0/4] hamradio: Fine-tuning for nine function implementations
From: David Miller @ 2017-05-09 14:43 UTC (permalink / raw)
To: elfring; +Cc: linux-hams, netdev, javier, jpr, linux-kernel, kernel-janitors
In-Reply-To: <4c5c939a-bd55-7b88-8e41-49f5544b7658@users.sourceforge.net>
You can feel free to continue submitting these changes, even though
people have asked you to back off on this, and that there is little to
no value to this churn.
But I personally am not going to apply any of your changes...
Especially since you keep posting even though people are asking you to
not make these changes.
You can ignore feedback like that, and you are explicitly being
notified that as a result, we can feel free to ignore you _too_.
People who submit kernel changes in the way you do waste a lot of
people's valuable time which could be spent on fixing real bugs,
implementing new important features, adding new documentation to
improve the understanding of the kernel for everyone, etc.
But instead, that time is being invested to reviewing your extremely
low value patches, many of which are undesirable.
I will not stand for it as the networking maintainer and am going to
ignore everything you submit until your approach and attitude towards
kernel patch submission _fundamentally_ (not temporarily, or for one
specific set of patches) changes.
Thank you.
^ permalink raw reply
* Re: DQL and TCQ_F_CAN_BYPASS destroy performance under virtualizaiton (Was: "Re: net_sched strange in 4.11")
From: Stefan Hajnoczi @ 2017-05-09 15:11 UTC (permalink / raw)
To: Anton Ivanov; +Cc: David S. Miller, netdev, Michael S. Tsirkin, jasowang
In-Reply-To: <27ae4e1c-7c6c-14c2-f3a4-9d0b1265d034@cambridgegreys.com>
[-- Attachment #1: Type: text/plain, Size: 2995 bytes --]
On Tue, May 09, 2017 at 08:46:46AM +0100, Anton Ivanov wrote:
> I have figured it out. Two issues.
>
> 1) skb->xmit_more is hardly ever set under virtualization because the qdisc
> is usually bypassed because of TCQ_F_CAN_BYPASS. Once TCQ_F_CAN_BYPASS is
> set a virtual NIC driver is not likely see skb->xmit_more (this answers my
> "how does this work at all" question).
>
> 2) If that flag is turned off (I patched sched_generic to turn it off in
> pfifo_fast while testing), DQL keeps xmit_more from being set. If the driver
> is not DQL enabled xmit_more is never ever set. If the driver is DQL enabled
> the queue is adjusted to ensure xmit_more stops happening within 10-15 xmit
> cycles.
>
> That is plain *wrong* for virtual NICs - virtio, emulated NICs, etc. There,
> the BIG cost is telling the hypervisor that it needs to "kick" the packets.
> The cost of putting them into the vNIC buffers is negligible. You want
> xmit_more to happen - it makes between 50% and 300% (depending on vNIC
> design) difference. If there is no xmit_more the vNIC will immediately
> "kick" the hypervisor and try to signal that the packet needs to move
> straight away (as for example in virtio_net).
>
> In addition to that, the perceived line rate is proportional to this cost,
> so I am not sure that the current dql math holds. In fact, I think it does
> not - it is trying to adjust something which influences the perceived line
> rate.
>
> So - how do we turn BOTH bypass and DQL adjustment while under
> virtualization and set them to be "always qdisc" + "always xmit_more
> allowed"
>
> A.
>
> P.S. Cc-ing virtio maintainer
CCing Michael Tsirkin and Jason Wang, who are the core virtio and
virtio-net maintainers. (I maintain the vsock driver - it's unrelated
to this discussion.)
>
> A.
>
>
> On 08/05/17 08:15, Anton Ivanov wrote:
> > Hi all,
> >
> > I was revising some of my old work for UML to prepare it for submission
> > and I noticed that skb->xmit_more does not seem to be set any more.
> >
> > I traced the issue as far as net/sched/sched_generic.c
> >
> > try_bulk_dequeue_skb() is never invoked (the drivers I am working on are
> > dql enabled so that is not the problem).
> >
> > More interestingly, if I put a breakpoint and debug output into
> > dequeue_skb() around line 147 - right before the bulk: tag that skb
> > there is always NULL. ???
> >
> > Similarly, debug in pfifo_fast_dequeue shows only NULLs being dequeued.
> > Again - ???
> >
> > First and foremost, I apologize for the silly question, but how can this
> > work at all? I see the skbs showing up at the driver level, why are
> > NULLs being returned at qdisc dequeue and where do the skbs at the
> > driver level come from?
> >
> > Second, where should I look to fix it?
> >
> > A.
> >
>
>
> --
> Anton R. Ivanov
>
> Cambridge Greys Limited, England company No 10273661
> http://www.cambridgegreys.com/
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 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