From patchwork Thu Apr 4 00:22:04 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kumar Kartikeya Dwivedi X-Patchwork-Id: 13616853 X-Patchwork-Delegate: bpf@iogearbox.net Received: from mail-ed1-f68.google.com (mail-ed1-f68.google.com [209.85.208.68]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EA1CC367 for ; Thu, 4 Apr 2024 00:22:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.68 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712190164; cv=none; b=Gwr8jsNXTNE4elHUrb+kbVd9SSrZJNg+B0hjUhgsbNYPKLt0wKGUlPuCwp1dbEvOjOmF6UZpF4ugzQs1Db99Bz3PPytKn4DbzUgTyyODy+ZfH/BDO16XR8Kc5WHEYT0Nb+ae7+/STXQlnSfLGILCvT33Cb6Kbqx+ygevLwFQ2FY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712190164; c=relaxed/simple; bh=Lm4/A635a5jSk8NJyf5HfokloGdMpygMez72pHq0QfU=; h=MIME-Version:From:Date:Message-ID:Subject:To:Cc:Content-Type; b=MwKh0sMPccU1zWE0mgaMUiRX8AQN/C0ELPF17S2lPh+7Q051Q/bPn7XZUzyt38bV+sgwS3/B6JhVmo44O0RV02BZFlqXkU8sFOwpEgPd5FuIQvUx/txZwyG/ieCGcXxKrOjP50lyFJyAXJhq0IArKgq1ukbSWzB9/MDTZFm7MzU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=JebNQw3F; arc=none smtp.client-ip=209.85.208.68 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="JebNQw3F" Received: by mail-ed1-f68.google.com with SMTP id 4fb4d7f45d1cf-56c404da0ebso560499a12.0 for ; Wed, 03 Apr 2024 17:22:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1712190161; x=1712794961; darn=vger.kernel.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=k9aPL42zG9w344myxSIGK4U/+YUYYhuVeLwo1JYLtUY=; b=JebNQw3Fu04CjKMn2PeA7YeW0HPuviAAjTiazLt6ljj0/oHnHLqFUcSqFeallETJRR R196WKWkMGVPx61Ktjx8c0Mnfevnf4CRvPJFlgVJ4WkLEI17NHgepM+3lPNgCPKnTPE3 b3A9zyjbgPHw0RCwodIRaqqu7t3P6mOZ7g+cUdB1W0+ktDrTy6trysRRWpn++jhvAU5R vdkYK7jNFrKQpdG7uwgRXuQBlVQk0unJO9vDgRN+sFYQgwLyX88hYgucLL0JpWuQOBYT 2/T3acM4EENNfA06vKCj2a5oQcVsoVfCjIYk5SAbSdy3CMcKnAFv2h/OKlARrzMhLjOM QPWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712190161; x=1712794961; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=k9aPL42zG9w344myxSIGK4U/+YUYYhuVeLwo1JYLtUY=; b=KuSkkMcJ0ASe/a33WVhEaruTe3OY+FveAHN1jchwOtP7jI83XkY4RzpQq6VgC1Pcaj UcHlrFGSARJOifvfv8ZECx/q9Wy/GI4TOTkNVn/Oa8ucJMpRyKmESYjRR6tSH7rAgMNy QBNumw9KC8J9m2/S/Oimx1TeTaGJOHXjV/FfcSuAa7Jkc6cxn0xit1y2anOWCpiZE1Td 2qtgOfQgbu/STBlcIFzNSW+w9vjD46jSAnef3tF/W+C7fwm7fCSOnSyDIIkcUyBo9Lw9 UICiSHt7k/DgN+Hy28Z6HRPFTUT4/QOOjRCqPH+DheXHekprfNB2ea1q/nuug+TQwjv2 py7A== X-Forwarded-Encrypted: i=1; AJvYcCWaIUe8LEACU3jNB2BW0oMutulIU76BUspAs67rkPSE8/G2CYsQIZibiC1gvXh3pRKzUC4y2HLw9ucFCxI3Kr1sCpnC X-Gm-Message-State: AOJu0YwjhM9MEwYGMF8otr1EoYBux6bZfnoKgQHVcaNAuJaGZiUNJ7m5 gwYcTxVvzRqoUaZ7QatkSC8iXcJSKuk50YN8uFHV0+VdiMs5hlyWJBBu1I1t2QX2AHpYHuMZNud I+QLiBAwGC3jIxNlIBcmg+kuzh7Q= X-Google-Smtp-Source: AGHT+IEWehhFlMuuTffxRMmhGABa7p6gmAXdm5g6b1YNhfQJryg/dTl6MrhDXjyggVvHeBa0NWUPqLwyB46B2rI7Rv0= X-Received: by 2002:a50:9b5a:0:b0:56e:603:9fc9 with SMTP id a26-20020a509b5a000000b0056e06039fc9mr599313edj.3.1712190161055; Wed, 03 Apr 2024 17:22:41 -0700 (PDT) Precedence: bulk X-Mailing-List: bpf@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Kumar Kartikeya Dwivedi Date: Thu, 4 Apr 2024 02:22:04 +0200 Message-ID: Subject: [Bug Report] Packet drops after trimming skb using bpf_skb_adjust_room To: John Fastabend , bpf Cc: Kashyap Sanidhya , rishabh.iyer@berkley.edu, Daniel Borkmann , Jakub Sitnicki X-Patchwork-Delegate: bpf@iogearbox.net Hello John, For background, I have a sk_skb program where for certain flows, a subset of request handling logic (on Rx) that is typically done in user space is handed over to a BPF program for doing it in-kernel. Since the protocol is over TCP, I'm using sk_skb programs to handle these requests and generate responses from within the kernel. The user space application will decide which flow should be added to the sockmap on socket accept and then let it be handled in the kernel for some of the request types. However, when generating replies and trimming away extra bytes in the packet by using bpf_skb_adjust_room, I see packet drops because the strp_msg->full_len is not updated unless tls_sw_has_ctx_rx is true for the socket. Removing that condition makes everything work, but I am not sure that is the right fix. My understanding so far is that in sk_psock_backlog, sk_psock_handle_skb returns an error since the stm->full_len is not correct (and passed into it as len), and SK_PSOCK_TX_ENABLED gets cleared due to this error. Then, on the redirect side (sk_psock_skb_redirect), it does sock_drop because the test for the SK_PSOCK_TX_ENABLED flag fails, which does kfree_skb. I have attached a patch with a selftest (skb_adjust_room) that you can run to verify the problem. It should basically hang on the second read as the packet would be dropped by the kernel. The first read goes through fine. Applying the diff below allows the selftest to pass. I am not familiar with the code to know whether the fix below is correct, but it seems to resolve the problem for me (and I carried it for a while and ran my stuff on it until I got around to reporting this now). In case gmail screws up the formatting, I'm just removing the tls_sw_has_ctx_rx(skb->sk) conditional on the strp_msg->full_len update in sk_skb_adjust_room. - rxm->full_len += len_diff; - } + strp_msg(skb)->full_len += len_diff; return ret; } --- Thanks! diff --git a/net/core/filter.c b/net/core/filter.c index 524adf1fa6d0..e3009d0f3352 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -3622,11 +3622,7 @@ BPF_CALL_4(sk_skb_adjust_room, struct sk_buff *, skb, s32, len_diff, return -ENOMEM; __skb_pull(skb, len_diff_abs); } - if (tls_sw_has_ctx_rx(skb->sk)) { - struct strp_msg *rxm = strp_msg(skb); -