Date:   Sat, 28 Jan 2023 17:40:08 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Pietro Borrello <borrello@diag.uniroma1.it>
Cc:     Boris Pismenny <borisp@nvidia.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Vakul Garg <vakul.garg@nxp.com>,
        Cristiano Giuffrida <c.giuffrida@vu.nl>,
        "Bos, H.J." <h.j.bos@vu.nl>, Jakob Koschel <jkl820.git@gmail.com>,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next] net/tls: tls_is_tx_ready() checked list_entry
Message-ID: <Y9VP6Hw7jH0VelUX@corigine.com>
References: <20230128-list-entry-null-check-tls-v1-1-525bbfe6f0d0@diag.uniroma1.it>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230128-list-entry-null-check-tls-v1-1-525bbfe6f0d0@diag.uniroma1.it>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PagqR6aa7VW2PDD34DW4VfbdUqQVpyDZAfESCw6yeKvMnOUJ8hCFn09QOsP/?=
 =?us-ascii?Q?cewWIoVwE01SsH1Ll7auYl4MVsY16ODEniRy+nd6qYlhWeJMLj0reGVP5Gho?=
 =?us-ascii?Q?DJDVWT0ymO1GLCztPGEegYDWeyCzsvKTzqbGwpgyzbioeSWZk8Q+Iu58N7hn?=
 =?us-ascii?Q?7pt2tNR09/xU87NtDvjE9VfIlRqAHiASzhE7bPvoPSYmOzW6IOWonBDmE7SO?=
 =?us-ascii?Q?4QoIzb69TOoPmsUAKNMBUKqgCCy9W0dzp9w6daXNCo52VvIVtW33fh60g9HA?=
 =?us-ascii?Q?6UZkLo/QVDh+YSvqxMvFJDEuKNy6DTbKVxH4KRGyDbDrMPvJIsVP+F+5wOaC?=
 =?us-ascii?Q?+TX2YX+4O4tEf8KX2x5altQ6I5wUpOq6i7tAWoXBngmB+Vu7eU+HCvVK7WdO?=
 =?us-ascii?Q?wjcbMa6MkSTKBp825w/ZWtoeM5kQ6h2fkiXfmPGtNvYqT7khxM8Wq2I3G51y?=
 =?us-ascii?Q?1sO3tTy1lcBY2K67I3vNEiFjkU3VPi6LE1FXPWsrubndKAnSIdGTWLJ3KJh8?=
 =?us-ascii?Q?bhLAH9/q88m5x0gz0aH6Re7DlB4UUOX3+BABE8ZSF73XF5JN832g7B9yNLP4?=
 =?us-ascii?Q?uQyfvuGxnp8VJ40vkFPkxuDKSOQK8ONYUDcrLh1ItceoDyh21AN/vk8yr5+S?=
 =?us-ascii?Q?8B08z3PBXpr8PTYXtCvrs+vNwf2iOOG0Op9OQEJSh70NlFfib+N+HMqWgek1?=
 =?us-ascii?Q?H+5zqOUokthwQxQKxPJziT09BGW7umbASbM/WTVHWFut1Zl0iWBRibUNinzO?=
 =?us-ascii?Q?Js6rLMOtzwYbTw2hblT1/cnFs3wcNkQLw4X/oC0pJCFWtskkUchq9720hsox?=
 =?us-ascii?Q?Pl1dm5nJFH7CRT1gXUtgQjIvNeQBwkjjv9b72HMGzPSnPFsj1I5p01lxdz3L?=
 =?us-ascii?Q?79WFQutp95uqtcFbthSz7el3Ydp3TBbmfOdeYhyrOKQ6s0EzNRawxlZhBDNF?=
 =?us-ascii?Q?LiZlP2QY7AvNF5WCoapfMn1KRySuzUHhlRLwjQ8RyeR4DkeqZuJRY3sxKJDY?=
 =?us-ascii?Q?OGltNG+lGKo8EtavHef6jF7qUmjWVMbp04w8TRs56B9eiZxHRFUfz+cVEK+l?=
 =?us-ascii?Q?EvSiv45z6fopXrMGvaG3m3CIV41LPHrV7FcnL4fEF4F2DZJxctU+Spxy+sUm?=
 =?us-ascii?Q?5H4Vb6GmY+2sU27TysI7psIYsRzThdu401hXVrlU+MHcPgfx/0X6P/EY48CE?=
 =?us-ascii?Q?QFIZnO2GNFd3bMju5c+hl5Md4AqLSk3QkfhloNPlmQrPAiNWDRgjbZTl71Ao?=
 =?us-ascii?Q?ZAdm2rcTxeVSVZseZsUu17TqsXz/2RStuxJcqi/qK/kmMOG+SL9wHJNDNDIA?=
 =?us-ascii?Q?XVkXhoVKOEG39A7ZWP2L+xzOFlw1cTKR4XR6GA86bufIjUHNUkThSpCOqZD6?=
 =?us-ascii?Q?8HxGgfgyUIwkwobqf8Gq5BbkMHnqy1EnkZmN1AsI+OxgMID1hENWmC/ORMll?=
 =?us-ascii?Q?qsNSR3fM81Q14NHOLhB27udys0JdbXBsqyazACmrQ5FSkcespv6+jAa8EeF/?=
 =?us-ascii?Q?BcxaqUuVsgALlKYikYmzu4h6XZQECbrYBlImuDqbhCN0GSRjMb3fmC/VpT7r?=
 =?us-ascii?Q?7FW1lp2NnVJCJmd3htuH8bMCmNFM7n1LN3kzyMfBrsadOKuVReQ0c9unj+ly?=
 =?us-ascii?Q?V+PCnxGirz55ROOftQNy768aw+xpFC4NyAGdJj6BIlAHiFNPHXDIIBElyTdW?=
 =?us-ascii?Q?jfxl0w=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9c9ddbe5-fd0a-4a2b-9dbd-08db014e555e
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jan 2023 16:40:15.6429
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: SJzcP+4OGBXNeyuEH2W7ZHyOQfbwU/CU3YkslMG89MKvx6hfhLS9phTDwijZ/1ByR26CcsnEp1jZsB9jaNorEeXCZU3utypr9FZg46wc4kM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR13MB4937
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Jan 28, 2023 at 04:29:17PM +0000, Pietro Borrello wrote:
> tls_is_tx_ready() checks that list_first_entry() does not return NULL.
> This condition can never happen. For empty lists, list_first_entry()
> returns the list_entry() of the head, which is a type confusion.
> Use list_first_entry_or_null() which returns NULL in case of empty
> lists.
> 
> Fixes: a42055e8d2c3 ("net/tls: Add support for async encryption of records for performance")
> Signed-off-by: Pietro Borrello <borrello@diag.uniroma1.it>
> ---
>  net/tls/tls_sw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 9ed978634125..a83d2b4275fa 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -2427,7 +2427,7 @@ static bool tls_is_tx_ready(struct tls_sw_context_tx *ctx)
>  {
>  	struct tls_rec *rec;
>  
> -	rec = list_first_entry(&ctx->tx_list, struct tls_rec, list);
> +	rec = list_first_entry_or_null(&ctx->tx_list, struct tls_rec, list);
>  	if (!rec)
>  		return false;

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.