Merge branch 'net-skb_put_padto-fixes'

Eric Dumazet says:

====================
net: skb_put_padto() fixes

sysbot reported a bug in qrtr leading to use-after-free.

First patch fixes the issue.

Second patch addes __must_check attribute to avoid similar
issues in the future.
====================

Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
David S. Miller 2020-09-09 11:04:39 -07:00
commit 0ddaa27803
2 changed files with 15 additions and 13 deletions

View File

@ -3223,7 +3223,8 @@ static inline int skb_padto(struct sk_buff *skb, unsigned int len)
* is untouched. Otherwise it is extended. Returns zero on * is untouched. Otherwise it is extended. Returns zero on
* success. The skb is freed on error if @free_on_error is true. * success. The skb is freed on error if @free_on_error is true.
*/ */
static inline int __skb_put_padto(struct sk_buff *skb, unsigned int len, static inline int __must_check __skb_put_padto(struct sk_buff *skb,
unsigned int len,
bool free_on_error) bool free_on_error)
{ {
unsigned int size = skb->len; unsigned int size = skb->len;
@ -3247,7 +3248,7 @@ static inline int __skb_put_padto(struct sk_buff *skb, unsigned int len,
* is untouched. Otherwise it is extended. Returns zero on * is untouched. Otherwise it is extended. Returns zero on
* success. The skb is freed on error. * success. The skb is freed on error.
*/ */
static inline int skb_put_padto(struct sk_buff *skb, unsigned int len) static inline int __must_check skb_put_padto(struct sk_buff *skb, unsigned int len)
{ {
return __skb_put_padto(skb, len, true); return __skb_put_padto(skb, len, true);
} }

View File

@ -332,8 +332,7 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
{ {
struct qrtr_hdr_v1 *hdr; struct qrtr_hdr_v1 *hdr;
size_t len = skb->len; size_t len = skb->len;
int rc = -ENODEV; int rc, confirm_rx;
int confirm_rx;
confirm_rx = qrtr_tx_wait(node, to->sq_node, to->sq_port, type); confirm_rx = qrtr_tx_wait(node, to->sq_node, to->sq_port, type);
if (confirm_rx < 0) { if (confirm_rx < 0) {
@ -357,15 +356,17 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
hdr->size = cpu_to_le32(len); hdr->size = cpu_to_le32(len);
hdr->confirm_rx = !!confirm_rx; hdr->confirm_rx = !!confirm_rx;
skb_put_padto(skb, ALIGN(len, 4) + sizeof(*hdr)); rc = skb_put_padto(skb, ALIGN(len, 4) + sizeof(*hdr));
if (!rc) {
mutex_lock(&node->ep_lock); mutex_lock(&node->ep_lock);
rc = -ENODEV;
if (node->ep) if (node->ep)
rc = node->ep->xmit(node->ep, skb); rc = node->ep->xmit(node->ep, skb);
else else
kfree_skb(skb); kfree_skb(skb);
mutex_unlock(&node->ep_lock); mutex_unlock(&node->ep_lock);
}
/* Need to ensure that a subsequent message carries the otherwise lost /* Need to ensure that a subsequent message carries the otherwise lost
* confirm_rx flag if we dropped this one */ * confirm_rx flag if we dropped this one */
if (rc && confirm_rx) if (rc && confirm_rx)