Skip to content

Commit d1d17a3

Browse files
lxinklassert
authored andcommitted
esp: remove the skb from the chain when it's enqueued in cryptd_wq
Xiumei found a panic in esp offload: BUG: unable to handle kernel NULL pointer dereference at 0000000000000020 RIP: 0010:esp_output_done+0x101/0x160 [esp4] Call Trace: ? esp_output+0x180/0x180 [esp4] cryptd_aead_crypt+0x4c/0x90 cryptd_queue_worker+0x6e/0xa0 process_one_work+0x1a7/0x3b0 worker_thread+0x30/0x390 ? create_worker+0x1a0/0x1a0 kthread+0x112/0x130 ? kthread_flush_work_fn+0x10/0x10 ret_from_fork+0x35/0x40 It was caused by that skb secpath is used in esp_output_done() after it's been released elsewhere. The tx path for esp offload is: __dev_queue_xmit()-> validate_xmit_skb_list()-> validate_xmit_xfrm()-> esp_xmit()-> esp_output_tail()-> aead_request_set_callback(esp_output_done) <--[1] crypto_aead_encrypt() <--[2] In [1], .callback is set, and in [2] it will trigger the worker schedule, later on a kernel thread will call .callback(esp_output_done), as the call trace shows. But in validate_xmit_xfrm(): skb_list_walk_safe(skb, skb2, nskb) { ... err = x->type_offload->xmit(x, skb2, esp_features); [esp_xmit] ... } When the err is -EINPROGRESS, which means this skb2 will be enqueued and later gets encrypted and sent out by .callback later in a kernel thread, skb2 should be removed fromt skb chain. Otherwise, it will get processed again outside validate_xmit_xfrm(), which could release skb secpath, and cause the panic above. This patch is to remove the skb from the chain when it's enqueued in cryptd_wq. While at it, remove the unnecessary 'if (!skb)' check. Fixes: 3dca3f3 ("xfrm: Separate ESP handling from segmentation for GRO packets.") Reported-by: Xiumei Mu <[email protected]> Signed-off-by: Xin Long <[email protected]> Signed-off-by: Steffen Klassert <[email protected]>
1 parent edf0d28 commit d1d17a3

File tree

1 file changed

+4
-4
lines changed

1 file changed

+4
-4
lines changed

net/xfrm/xfrm_device.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,8 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
7878
int err;
7979
unsigned long flags;
8080
struct xfrm_state *x;
81-
struct sk_buff *skb2, *nskb;
8281
struct softnet_data *sd;
82+
struct sk_buff *skb2, *nskb, *pskb = NULL;
8383
netdev_features_t esp_features = features;
8484
struct xfrm_offload *xo = xfrm_offload(skb);
8585
struct sec_path *sp;
@@ -168,14 +168,14 @@ struct sk_buff *validate_xmit_xfrm(struct sk_buff *skb, netdev_features_t featur
168168
} else {
169169
if (skb == skb2)
170170
skb = nskb;
171-
172-
if (!skb)
173-
return NULL;
171+
else
172+
pskb->next = nskb;
174173

175174
continue;
176175
}
177176

178177
skb_push(skb2, skb2->data - skb_mac_header(skb2));
178+
pskb = skb2;
179179
}
180180

181181
return skb;

0 commit comments

Comments
 (0)