linux-kernel - Re: [PATCH v2] r8169: hardening and stability improvements

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <e461ef27-a6cf-4ed9-adec-e7d949958df9@lunn.ch>
Date: Sat, 30 Aug 2025 16:44:02 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Mohammad Amin Hosseini <moahmmad.hosseinii@...il.com>
Cc: hkallweit1@...il.com, nic_swsd@...ltek.com, andrew+netdev@...n.ch,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] r8169: hardening and stability improvements

> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index 9c601f271c02..66d7dcd8bf7b 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -3981,19 +3981,39 @@ static struct page *rtl8169_alloc_rx_data(struct rtl8169_private *tp,
>  	int node = dev_to_node(d);
>  	dma_addr_t mapping;
>  	struct page *data;
> +	gfp_t gfp_flags = GFP_KERNEL;
>  
> -	data = alloc_pages_node(node, GFP_KERNEL, get_order(R8169_RX_BUF_SIZE));
> -	if (!data)
> -		return NULL;
> +	/* Use atomic allocation in interrupt/atomic context */
> +	if (in_atomic() || irqs_disabled())
> +		gfp_flags = GFP_ATOMIC;

/*
 * Are we running in atomic context?  WARNING: this macro cannot
 * always detect atomic context; in particular, it cannot know about
 * held spinlocks in non-preemptible kernels.  Thus it should not be
 * used in the general case to determine whether sleeping is possible.
 * Do not use in_atomic() in driver code.
 */
#define in_atomic()	(preempt_count() != 0)

You need to explain why you ignored this last line, and why this is
safe in this context. For a change like this, which breaks the normal
rules, you want it in a patch of its own, so the explanation can be in
the commit message and it is then very clear why you are ignoring the
rules.

Looking at the rest of the patch, a lot of the changes are
questionable. Splitting it up will help you describe why you are
making each change, why it is needed.

    Andrew

---
pw-bot: cr