From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E1CE0C27C76 for ; Sat, 28 Jan 2023 16:57:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233212AbjA1Q5W (ORCPT ); Sat, 28 Jan 2023 11:57:22 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50860 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230110AbjA1Q5U (ORCPT ); Sat, 28 Jan 2023 11:57:20 -0500 Received: from mail-ed1-x535.google.com (mail-ed1-x535.google.com [IPv6:2a00:1450:4864:20::535]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 856182A178 for ; Sat, 28 Jan 2023 08:57:18 -0800 (PST) Received: by mail-ed1-x535.google.com with SMTP id s3so7349182edd.4 for ; Sat, 28 Jan 2023 08:57:18 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=diag.uniroma1.it; s=google; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=tt8Ha/RSXw0RfPBUqnD7ihG/77QJ3kclrfnxQeGKaOg=; b=vdySnnQpnG7PamHqJRpJHTk2/+pkWDwtlaaTwo/4ftut1SPbh2wsoeOBfyqSe97XJr mBy/7KQLHn305eIbrv15wtIC3jt677R6UlSH0zZLLGg/AOXKk5sofUt21qCLjX8Xqvpz ZRdX45XYwjwfNRYgk0ChwEm0M4J3VJXcUuOjg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=tt8Ha/RSXw0RfPBUqnD7ihG/77QJ3kclrfnxQeGKaOg=; b=SIa0pLMV1h/Zh0kXGZRwv7So8OOkh3BXJPXJK78PjHjlh4tV5mWXih9vqzCg5CODn3 47RoOuxW6O7yLCT+uhYMDkbpUG+lXqII08zb82uF1L17mACz/xHruRrpy8uw8JT0kSMS pWfWYROrIJTEn2nCutPqDpw0gEKth9ij1QLeqAWzlEph/Hp3qpoKBNLNB3fyRMHZVaoR wCEPZLsNpAqsNAsOzuiGwUbHkym7/Ebx5VdNuEB3yohm1sxe7QMDpPgg7q/qLEI3k5Il a/FZD+999G6N6j2hkuI3yoEauPSxwzQtI1L6sBnMb9kMqc8B+TlBY/WUMPdX4bAitICC C8qw== X-Gm-Message-State: AO0yUKXJx7a9asaYmWuEQ7KwPj891WcMWbCTMYHWW2PLn2RuZ23of/wh +0bNivR/zR01g243mQCzw2BATMaE8zUgRAAy4mpgCQ== X-Google-Smtp-Source: AK7set8gwJjmXGZ7qEK5w/UZ8ko/Bw99HYHzRiyxR2URnKVNJoJLwhG2Y0TJ+hLyQwxjUH1ZmX3ZHnlhwaLOabgBZQ8= X-Received: by 2002:a05:6402:552:b0:4a0:8fde:99b4 with SMTP id i18-20020a056402055200b004a08fde99b4mr3865536edx.32.1674925037155; Sat, 28 Jan 2023 08:57:17 -0800 (PST) MIME-Version: 1.0 References: <20230128-list-entry-null-check-tls-v1-1-525bbfe6f0d0@diag.uniroma1.it> In-Reply-To: From: Pietro Borrello Date: Sat, 28 Jan 2023 17:57:06 +0100 Message-ID: Subject: Re: [PATCH net-next] net/tls: tls_is_tx_ready() checked list_entry To: Simon Horman Cc: Boris Pismenny , John Fastabend , Jakub Kicinski , "David S. Miller" , Eric Dumazet , Paolo Abeni , Vakul Garg , Cristiano Giuffrida , "Bos, H.J." , Jakob Koschel , netdev@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, 28 Jan 2023 at 17:40, Simon Horman wrote: > > Hi Pietro, > > I agree this is correct. > > However, given that the code has been around for a while, > I feel it's relevant to ask if tx_list can ever be NULL. > If not, perhaps it's better to remove the error path entirely. Hi Simon, Thank you for your fast reply. The point is exactly that tx_list will never be NULL, as the list head will always be present. So the error, as is, will never be detected, resulting in type confusion. We found this with static analysis, so we have no way to say for sure that the list can never be empty on edge cases. As this is a type confusion, the errors are often sneaky and go undetected. As an example, the following bug we previously reported resulted in a type confusion on net code that went undetected for more than 20 years. Link: https://lore.kernel.org/all/9fcd182f1099f86c6661f3717f63712ddd1c676c.1674496737.git.marcelo.leitner@gmail.com/ In that case, we were able to create a PoC to demonstrate the issue where we leveraged the type confusion to bypass KASLR. In the end, this is the maintainer's call, but I would keep the check and correctly issue a list_first_entry_or_null() so that the check will work as intended as the added overhead is just a pointer comparison which would likely justify the cost of a more solid code. Otherwise, I can also submit a patch that entirely removes the check. Let me know what you prefer. Best regards, Pietro