netdev - Re:Re: [net-next,v7,7/8] net: wwan: hwsim: support NMEA port emulation

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: <273e24af.2e77.19be3aafec9.Coremail.slark_xiao@163.com>
Date: Thu, 22 Jan 2026 11:06:33 +0800 (CST)
From: "Slark Xiao" <slark_xiao@....com>
To: "Jakub Kicinski" <kuba@...nel.org>,
	"Sergey Ryazanov" <ryazanov.s.a@...il.com>,
	"Loic Poulain" <loic.poulain@....qualcomm.com>
Cc: quic_qianyu@...cinc.com, davem@...emloft.net,
	loic.poulain@....qualcomm.com, johannes@...solutions.net,
	netdev@...r.kernel.org, zaihan@...ealasia.net, johan@...nel.org,
	andrew+netdev@...n.ch, edumazet@...gle.com,
	linux-kernel@...r.kernel.org, dnlplm@...il.com,
	manivannan.sadhasivam@...aro.org, pabeni@...hat.com, mani@...nel.org,
	ryazanov.s.a@...il.com
Subject: Re:Re: [net-next,v7,7/8] net: wwan: hwsim: support NMEA port
 emulation


At 2026-01-16 10:44:41, "Jakub Kicinski" <kuba@...nel.org> wrote:
>This is an AI-generated review of your patch. The human sending this
>email has considered the AI review valid, or at least plausible.
>
>For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
>---
>net: wwan: hwsim: support NMEA port emulation
>
>This commit adds NMEA port emulation to the WWAN hardware simulator for
>GNSS port testing. The emulator generates GGA and RMC sentences every
>second via a timer callback.
>
>> diff --git a/drivers/net/wwan/wwan_hwsim.c b/drivers/net/wwan/wwan_hwsim.c
>> index 11d15dc39041..e4b1bbff9af2 100644
>> --- a/drivers/net/wwan/wwan_hwsim.c
>> +++ b/drivers/net/wwan/wwan_hwsim.c
>
>[ ... ]
>
>> +#if IS_ENABLED(CONFIG_GNSS)
>> +#define NMEA_MAX_LEN		82	/* Max sentence length */
>> +#define NMEA_TRAIL_LEN		5	/* '*' + Checksum + <CR><LF> */
>> +#define NMEA_MAX_DATA_LEN	(NMEA_MAX_LEN - NMEA_TRAIL_LEN)
>> +
>> +static __printf(2, 3)
>> +void wwan_hwsim_nmea_skb_push_sentence(struct sk_buff *skb,
>> +				       const char *fmt, ...)
>> +{
>> +	unsigned char *s, *p;
>> +	va_list ap;
>> +	u8 cs = 0;
>> +	int len;
>> +
>> +	s = skb_put(skb, NMEA_MAX_LEN + 1);	/* +'\0' */
>
>[ ... ]
>
>> +static void wwan_hwsim_nmea_emul_timer(struct timer_list *t)
>> +{
>> +	/* 43.74754722298909 N 11.25759835922875 E in DMM format */
>> +	static const unsigned int coord[4 * 2] = { 43, 44, 8528, 0,
>> +						   11, 15, 4559, 0 };
>> +	struct wwan_hwsim_port *port = timer_container_of(port, t, nmea_emul.timer);
>> +	struct sk_buff *skb;
>> +	struct tm tm;
>> +
>> +	time64_to_tm(ktime_get_real_seconds(), 0, &tm);
>> +
>> +	mod_timer(&port->nmea_emul.timer, jiffies + HZ);	/* 1 second */
>> +
>> +	skb = alloc_skb(NMEA_MAX_LEN * 2, GFP_KERNEL);	/* GGA + RMC */
>                       ^^^^^^^^^^^^^^     ^^^^^^^^^^
>
Hi Sergey, Loic,
Let's update it as :
+	skb = alloc_skb(NMEA_MAX_LEN * 2+2, GFP_ATOMIC);	/* GGA + RMC */

How do you think?


>Two questions about this allocation:
>
>1. The buffer size appears to be insufficient. The allocation requests
>NMEA_MAX_LEN * 2 = 164 bytes, but wwan_hwsim_nmea_skb_push_sentence()
>is called twice and each call uses skb_put(skb, NMEA_MAX_LEN + 1) which
>reserves 83 bytes per sentence. The total needed is 166 bytes, which
>exceeds the 164 bytes allocated. Would this cause skb_over_panic() to
>trigger on the second skb_put() call?
>
>2. This function is a timer callback registered via timer_setup() in
>wwan_hwsim_nmea_emul_start(). Timer callbacks run in softirq context
>where sleeping is not permitted. GFP_KERNEL allocations can sleep when
>memory pressure triggers reclaim. Should this be GFP_ATOMIC instead to
>avoid "scheduling while atomic" warnings?
>
>> +	if (!skb)
>> +		return;
>> +
>> +	wwan_hwsim_nmea_skb_push_sentence(skb,
>> +					  "$GPGGA,%02u%02u%02u.000,%02u%02u.%04u,%c,%03u%02u.%04u,%c,1,7,1.03,176.2,M,55.2,M,,",
>> +					  tm.tm_hour, tm.tm_min, tm.tm_sec,
>> +					  coord[0], coord[1], coord[2],
>> +					  coord[3] ? 'S' : 'N',
>> +					  coord[4], coord[5], coord[6],
>> +					  coord[7] ? 'W' : 'E');
>> +
>> +	wwan_hwsim_nmea_skb_push_sentence(skb,
>> +					  "$GPRMC,%02u%02u%02u.000,A,%02u%02u.%04u,%c,%03u%02u.%04u,%c,0.02,31.66,%02u%02u%02u,,,A",
>> +					  tm.tm_hour, tm.tm_min, tm.tm_sec,
>> +					  coord[0], coord[1], coord[2],
>> +					  coord[3] ? 'S' : 'N',
>> +					  coord[4], coord[5], coord[6],
>> +					  coord[7] ? 'W' : 'E',
>> +					  tm.tm_mday, tm.tm_mon + 1,
>> +					  (unsigned int)tm.tm_year - 100);
>> +
>> +	wwan_port_rx(port->wwan, skb);
>> +}
>
>[ ... ]