[PATCH v5] ptp: ocp: Limit signal/freq counts in summary output functions

[PATCH v5] ptp: ocp: Limit signal/freq counts in summary output functions

[Sagi Maimon]

The debugfs summary output could access uninitialized elements in
the freq_in[] and signal_out[] arrays, causing NULL pointer
dereferences and triggering a kernel Oops (page_fault_oops).
This patch adds u8 fields (nr_freq_in, nr_signal_out) to track the
number of initialized elements, with a maximum of 4 per array.
The summary output functions are updated to respect these limits,
preventing out-of-bounds access and ensuring safe array handling.

Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
---
Addressed comments from Vadim Fedorenko:
- https://www.spinics.net/lists/kernel/msg5683022.html
Addressed comments from Jakub Kicinski:
- https://www.spinics.net/lists/netdev/msg1091131.html
Changes since v4:
- remove fix from signal/freq show/store routines.
---
---
 drivers/ptp/ptp_ocp.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/drivers/ptp/ptp_ocp.c b/drivers/ptp/ptp_ocp.c
index 2ccdca4f6960..e63481f24238 100644
--- a/drivers/ptp/ptp_ocp.c
+++ b/drivers/ptp/ptp_ocp.c
@@ -315,6 +315,8 @@ struct ptp_ocp_serial_port {
 #define OCP_BOARD_ID_LEN		13
 #define OCP_SERIAL_LEN			6
 #define OCP_SMA_NUM			4
+#define OCP_SIGNAL_NUM			4
+#define OCP_FREQ_NUM			4
 
 enum {
 	PORT_GNSS,
@@ -342,8 +344,8 @@ struct ptp_ocp {
 	struct dcf_master_reg	__iomem *dcf_out;
 	struct dcf_slave_reg	__iomem *dcf_in;
 	struct tod_reg		__iomem *nmea_out;
-	struct frequency_reg	__iomem *freq_in[4];
-	struct ptp_ocp_ext_src	*signal_out[4];
+	struct frequency_reg	__iomem *freq_in[OCP_FREQ_NUM];
+	struct ptp_ocp_ext_src	*signal_out[OCP_SIGNAL_NUM];
 	struct ptp_ocp_ext_src	*pps;
 	struct ptp_ocp_ext_src	*ts0;
 	struct ptp_ocp_ext_src	*ts1;
@@ -378,10 +380,12 @@ struct ptp_ocp {
 	u32			utc_tai_offset;
 	u32			ts_window_adjust;
 	u64			fw_cap;
-	struct ptp_ocp_signal	signal[4];
+	struct ptp_ocp_signal	signal[OCP_SIGNAL_NUM];
 	struct ptp_ocp_sma_connector sma[OCP_SMA_NUM];
 	const struct ocp_sma_op *sma_op;
 	struct dpll_device *dpll;
+	int signals_nr;
+	int freq_in_nr;
 };
 
 #define OCP_REQ_TIMESTAMP	BIT(0)
@@ -2697,6 +2701,8 @@ ptp_ocp_fb_board_init(struct ptp_ocp *bp, struct ocp_resource *r)
 	bp->eeprom_map = fb_eeprom_map;
 	bp->fw_version = ioread32(&bp->image->version);
 	bp->sma_op = &ocp_fb_sma_op;
+	bp->signals_nr = 4;
+	bp->freq_in_nr = 4;
 
 	ptp_ocp_fb_set_version(bp);
 
@@ -2862,6 +2868,8 @@ 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;
 
 	/* Enable MAC serial port during initialisation */
 	iowrite32(1, &bp->board_config->mro50_serial_activate);
@@ -2888,6 +2896,8 @@ 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;
 
 	version = ioread32(&bp->image->version);
 	/* if lower 16 bits are empty, this is the fw loader. */
@@ -4008,7 +4018,7 @@ _signal_summary_show(struct seq_file *s, struct ptp_ocp *bp, int nr)
 {
 	struct signal_reg __iomem *reg = bp->signal_out[nr]->mem;
 	struct ptp_ocp_signal *signal = &bp->signal[nr];
-	char label[8];
+	char label[16];
 	bool on;
 	u32 val;
 
@@ -4031,7 +4041,7 @@ static void
 _frequency_summary_show(struct seq_file *s, int nr,
 			struct frequency_reg __iomem *reg)
 {
-	char label[8];
+	char label[16];
 	bool on;
 	u32 val;
 
@@ -4175,11 +4185,11 @@ ptp_ocp_summary_show(struct seq_file *s, void *data)
 	}
 
 	if (bp->fw_cap & OCP_CAP_SIGNAL)
-		for (i = 0; i < 4; i++)
+		for (i = 0; i < bp->signals_nr; i++)
 			_signal_summary_show(s, bp, i);
 
 	if (bp->fw_cap & OCP_CAP_FREQ)
-		for (i = 0; i < 4; i++)
+		for (i = 0; i < bp->freq_in_nr; i++)
 			_frequency_summary_show(s, i, bp->freq_in[i]);
 
 	if (bp->irig_out) {
-- 
2.47.0

Re: [PATCH v5] ptp: ocp: Limit signal/freq counts in summary output functions

[Simon Horman]

On Wed, May 14, 2025 at 10:35:41AM +0300, Sagi Maimon wrote:
> The debugfs summary output could access uninitialized elements in
> the freq_in[] and signal_out[] arrays, causing NULL pointer
> dereferences and triggering a kernel Oops (page_fault_oops).
> This patch adds u8 fields (nr_freq_in, nr_signal_out) to track the
> number of initialized elements, with a maximum of 4 per array.
> The summary output functions are updated to respect these limits,
> preventing out-of-bounds access and ensuring safe array handling.
> 
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
> ---
> Addressed comments from Vadim Fedorenko:
> - https://www.spinics.net/lists/kernel/msg5683022.html
> Addressed comments from Jakub Kicinski:
> - https://www.spinics.net/lists/netdev/msg1091131.html
> Changes since v4:
> - remove fix from signal/freq show/store routines.

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [PATCH v5] ptp: ocp: Limit signal/freq counts in summary output functions

[Vadim Fedorenko]

On 14/05/2025 08:35, Sagi Maimon wrote:
> The debugfs summary output could access uninitialized elements in
> the freq_in[] and signal_out[] arrays, causing NULL pointer
> dereferences and triggering a kernel Oops (page_fault_oops).
> This patch adds u8 fields (nr_freq_in, nr_signal_out) to track the
> number of initialized elements, with a maximum of 4 per array.
> The summary output functions are updated to respect these limits,
> preventing out-of-bounds access and ensuring safe array handling.
> 
> Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
> ---
> Addressed comments from Vadim Fedorenko:
> - https://www.spinics.net/lists/kernel/msg5683022.html
> Addressed comments from Jakub Kicinski:
> - https://www.spinics.net/lists/netdev/msg1091131.html
> Changes since v4:
> - remove fix from signal/freq show/store routines.
> ---

LGTM, Thanks!

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>

Re: [PATCH v5] ptp: ocp: Limit signal/freq counts in summary output functions

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 14 May 2025 10:35:41 +0300 you wrote:
> The debugfs summary output could access uninitialized elements in
> the freq_in[] and signal_out[] arrays, causing NULL pointer
> dereferences and triggering a kernel Oops (page_fault_oops).
> This patch adds u8 fields (nr_freq_in, nr_signal_out) to track the
> number of initialized elements, with a maximum of 4 per array.
> The summary output functions are updated to respect these limits,
> preventing out-of-bounds access and ensuring safe array handling.
> 
> [...]

Here is the summary with links:
  - [v5] ptp: ocp: Limit signal/freq counts in summary output functions
    https://git.kernel.org/netdev/net/c/c9e455581e2b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html