[PATCH v1] pppoe: Add PPP interface name to /proc/net/pppoe output

[PATCH v1] pppoe: Add PPP interface name to /proc/net/pppoe output

[Pavan Kumar]

The pppoe_seq_show() function displays session ID, remote MAC address,
and the underlying Ethernet device, but omits the associated PPP
interface name. This makes it difficult to correlate PPPoE sessions
with their respective pppX interfaces.

Retrieve the interface name using ppp_dev_name() and append it as a
new "PPP-Device" column in the /proc/net/pppoe output.

Signed-off-by: Pavan Kumar <pavan.m@oss.qualcomm.com>
---
 drivers/net/ppp/pppoe.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 6874a1a8edaf..cb31f5db43e8 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -963,17 +963,19 @@ static int pppoe_seq_show(struct seq_file *seq, void *v)
 {
 	struct pppox_sock *po;
 	char *dev_name;
+	char *ppp_name;
 
 	if (v == SEQ_START_TOKEN) {
-		seq_puts(seq, "Id       Address              Device\n");
+		seq_puts(seq, "Id       Address              Device   PPP-Device\n");
 		goto out;
 	}
 
 	po = v;
 	dev_name = po->pppoe_pa.dev;
+	ppp_name = ppp_dev_name(&po->chan);
 
-	seq_printf(seq, "%08X %pM %8s\n",
-		po->pppoe_pa.sid, po->pppoe_pa.remote, dev_name);
+	seq_printf(seq, "%08X %pM %8s %10s\n",
+		   po->pppoe_pa.sid, po->pppoe_pa.remote, dev_name, ppp_name ? ppp_name : "");
 out:
 	return 0;
 }

[PATCH v1] pppoe: Add PPP interface name to /proc/net/pppoe output

[Pavan Kumar]

The pppoe_seq_show() function displays session ID, remote MAC address,
and the underlying Ethernet device, but omits the associated PPP
interface name. This makes it difficult to correlate PPPoE sessions
with their respective pppX interfaces.

Retrieve the interface name using ppp_dev_name() and append it as a
new "PPP-Device" column in the /proc/net/pppoe output.

Signed-off-by: Pavan Kumar <pavan.m@oss.qualcomm.com>
---
 drivers/net/ppp/pppoe.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 6874a1a8edaf..cb31f5db43e8 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -963,17 +963,19 @@ static int pppoe_seq_show(struct seq_file *seq, void *v)
 {
 	struct pppox_sock *po;
 	char *dev_name;
+	char *ppp_name;
 
 	if (v == SEQ_START_TOKEN) {
-		seq_puts(seq, "Id       Address              Device\n");
+		seq_puts(seq, "Id       Address              Device   PPP-Device\n");
 		goto out;
 	}
 
 	po = v;
 	dev_name = po->pppoe_pa.dev;
+	ppp_name = ppp_dev_name(&po->chan);
 
-	seq_printf(seq, "%08X %pM %8s\n",
-		po->pppoe_pa.sid, po->pppoe_pa.remote, dev_name);
+	seq_printf(seq, "%08X %pM %8s %10s\n",
+		   po->pppoe_pa.sid, po->pppoe_pa.remote, dev_name, ppp_name ? ppp_name : "");
 out:
 	return 0;
 }

Re: [PATCH v1] pppoe: Add PPP interface name to /proc/net/pppoe output

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260728070636.3462165-1-pavan.m@oss.qualcomm.com?part=1
---
--- Patch [1]: [PATCH v1] pppoe: Add PPP interface name to /proc/net/pppoe output ---
commit fe1f42402d1a0992d1fdaf152ef4812c313dc548
Author: Pavan Kumar <pavan.m@oss.qualcomm.com>
pppoe: Add PPP interface name to /proc/net/pppoe output

This patch modifies the pppoe_seq_show() function to include the associated
PPP interface name in the /proc/net/pppoe output. It retrieves the interface
name using ppp_dev_name() and appends it as a new "PPP-Device" column to 
help correlate PPPoE sessions with their pppX interfaces.

> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 6874a1a8edaf1..cb31f5db43e81 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -963,17 +963,19 @@ static int pppoe_seq_show(struct seq_file *seq, void *v)
>  {
>  	struct pppox_sock *po;
>  	char *dev_name;
> +	char *ppp_name;
>  
>  	if (v == SEQ_START_TOKEN) {
> -		seq_puts(seq, "Id       Address              Device\n");
> +		seq_puts(seq, "Id       Address              Device   PPP-Device\n");
>  		goto out;
>  	}
>  
>  	po = v;
>  	dev_name = po->pppoe_pa.dev;
> +	ppp_name = ppp_dev_name(&po->chan);

[Severity: Critical]
Can this call lead to a wild pointer dereference and a kernel panic?

Reading /proc/net/pppoe invokes pppoe_seq_show() under rcu_read_lock().
However, it appears pppoe_connect() might modify po->chan without RCU
synchronization in a couple of racing scenarios.

First, during a rebind, pppoe_connect() unlinks the socket via delete_item(),
but then immediately zeroes the channel without waiting for a grace period:

drivers/net/ppp/pppoe.c:pppoe_connect() {
    ...
    delete_item(pn, po->pppoe_pa.sid, po->pppoe_pa.remote,
                po->pppoe_ifindex);
    ...
    memset(&po->chan, 0, sizeof(po->chan));
    ...
}

Would a concurrent /proc reader observe a partially zeroed (torn) chan->ppp
pointer here?

Second, during a new connection, __set_item() exposes the socket to
/proc readers before ppp_register_net_channel() initializes it:

drivers/net/ppp/pppoe.c:pppoe_connect() {
    ...
    error = __set_item(pn, po);
    ...
    ppp_register_net_channel(dev_net(dev), &po->chan);
    ...
}

Since ppp_register_net_channel() assigns the pointer with a plain store:

drivers/net/ppp/ppp_generic.c:ppp_register_net_channel() {
    ...
    chan->ppp = pch;
    ...
}

And ppp_dev_name() reads it via a plain load:

drivers/net/ppp/ppp_generic.c:ppp_dev_name() {
    ...
    struct channel *pch = chan->ppp;
    ...
}

Could readers on weakly-ordered architectures observe the new pointer
but read uninitialized memory for its contents?

Since /proc/net/pppoe is world-readable, could an unprivileged user 
continuously read this file while connections are managed, reliably
triggering a local denial of service?

>  
> -	seq_printf(seq, "%08X %pM %8s\n",
> -		po->pppoe_pa.sid, po->pppoe_pa.remote, dev_name);
> +	seq_printf(seq, "%08X %pM %8s %10s\n",
> +		   po->pppoe_pa.sid, po->pppoe_pa.remote, dev_name, ppp_name ? ppp_name : "");
>  out:
>  	return 0;
>  }