linux-kernel - Re: [PATCH v2] ptp: ocp: Limit SMA/signal/freq counts in show/store functions

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: <20250509150157.6cdf620c@kernel.org>
Date: Fri, 9 May 2025 15:01:57 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Sagi Maimon <maimon.sagi@...il.com>
Cc: jonathan.lemon@...il.com, vadim.fedorenko@...ux.dev,
 richardcochran@...il.com, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, pabeni@...hat.com, linux-kernel@...r.kernel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH v2] ptp: ocp: Limit SMA/signal/freq counts in show/store
 functions

On Thu,  8 May 2025 10:19:01 +0300 Sagi Maimon wrote:
> The sysfs show/store operations could access uninitialized elements in
> the freq_in[], signal_out[], and sma[] arrays, leading to NULL pointer
> dereferences. This patch introduces u8 fields (nr_freq_in, nr_signal_out,
> nr_sma) to track the actual number of initialized elements, capping the
> maximum at 4 for each array. The affected show/store functions are updated to

This line is too long. I think the recommended limit for commit message
is / was 72 or 74 chars.

> respect these limits, preventing out-of-bounds access and ensuring safe
> array handling.

What do you mean by out-of-bounds access here. Is there any access with
index > 4 possible? Or just with index > 1 for Adva?

We need more precise information about the problem to decide if this is
a fix or an improvement 

> +	bp->sma_nr  = 4;

nit: double space in all the sma_nr assignments

>  
>  	ptp_ocp_fb_set_version(bp);
>  
> @@ -2862,6 +2870,9 @@ ptp_ocp_art_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
>  	bp->fw_version = ioread32(&bp->reg->version);
>  	bp->fw_tag = 2;
>  	bp->sma_op = &ocp_art_sma_op;
> +	bp->signals_nr = 4;
> +	bp->freq_in_nr = 4;
> +	bp->sma_nr  = 4;
>  
>  	/* Enable MAC serial port during initialisation */
>  	iowrite32(1, &bp->board_config->mro50_serial_activate);
> @@ -2888,6 +2899,9 @@ ptp_ocp_adva_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
>  	bp->flash_start = 0xA00000;
>  	bp->eeprom_map = fb_eeprom_map;
>  	bp->sma_op = &ocp_adva_sma_op;
> +	bp->signals_nr = 2;
> +	bp->freq_in_nr = 2;
> +	bp->sma_nr  = 2;
>  
>  	version = ioread32(&bp->image->version);
>  	/* if lower 16 bits are empty, this is the fw loader. */
> @@ -3002,6 +3016,9 @@ ptp_ocp_sma_show(struct ptp_ocp *bp, int sma_nr, char *buf,
>  	const struct ocp_selector * const *tbl;
>  	u32 val;
>  
> +	if (sma_nr > bp->sma_nr)
> +		return 0;

Why are you returning 0 and not an error?

As a matter of fact why register the sysfs files for things which don't
exists?
-- 
pw-bot: cr