git: 08ec14fecf6a - stable/13 - sppp: Fix getting wrong spppreq cmd from ioctl

From: Zhenlei Huang <zlei_at_FreeBSD.org>
Date: Tue, 14 Jan 2025 10:58:26 UTC
The branch stable/13 has been updated by zlei:

URL: https://cgit.FreeBSD.org/src/commit/?id=08ec14fecf6a93c0321c31ba1f0b04db6b888f16

commit 08ec14fecf6a93c0321c31ba1f0b04db6b888f16
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2025-01-14 10:56:49 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2025-01-14 10:56:49 +0000

    sppp: Fix getting wrong spppreq cmd from ioctl
    
    ifr->ifr_data is supposed to point to a struct spppreq. The first member
    cmd of struct spppreq is int type. It was pre-read via `fueword()` before
    a full fetching. Unfortunately an user space `struct spppreq spr` may not
    be zeroed explicitly, on 64bit architectures `fueword()` reads 64bit word
    thus the garbage (extra 4 bytes) may be read into kernel space (subcmd).
    
    Prior to f9d8181868ee, `subcmd` was declared as int and assigned from
    `fuword()` and was implicitly converted from long to int. On 64bit little
    endian architectures the implicitly conversion overflows (undefined
    bahavior) which happen to trash the garbage (the extra 4 bytes, high
    32 bits) and worked, but no luck on 64bit big endian architectures.
    
    Since f9d8181868ee `subcmd` was changed to u_long then there is no
    conversion so we end up mismatching `subcmd` with user space's `cmd`.
    
    It is also a bit hackish to get the value of cmd via `fueword()`, instead
    we refer to it directly from spr->cmd.
    
    This is a direct commit to stable/13 as sppp(4) no longer exists in main
    and stable/14.
    
    PR:             173002
    Reviewed by:    glebius (previous version)
    Fixes:  f9d8181868ee Fixed yet more ioctl breakage due to the type of ...
    Differential Revision:  https://reviews.freebsd.org/D47335
---
 sys/net/if_spppsubr.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/sys/net/if_spppsubr.c b/sys/net/if_spppsubr.c
index ed198c59bbfe..269a6ef1d0da 100644
--- a/sys/net/if_spppsubr.c
+++ b/sys/net/if_spppsubr.c
@@ -5044,10 +5044,10 @@ sppp_suggest_ip6_addr(struct sppp *sp, struct in6_addr *suggest)
 static int
 sppp_params(struct sppp *sp, u_long cmd, void *data)
 {
-	u_long subcmd;
+	int subcmd __diagused;
 	struct ifreq *ifr = (struct ifreq *)data;
 	struct spppreq *spr;
-	int rv = 0;
+	int rv;
 
 	if ((spr = malloc(sizeof(struct spppreq), M_TEMP, M_NOWAIT)) == NULL)
 		return (EAGAIN);
@@ -5056,7 +5056,7 @@ sppp_params(struct sppp *sp, u_long cmd, void *data)
 	 * Check the cmd word first before attempting to fetch all the
 	 * data.
 	 */
-	rv = fueword(ifr_data_get_ptr(ifr), &subcmd);
+	rv = fueword32(ifr_data_get_ptr(ifr), &subcmd);
 	if (rv == -1) {
 		rv = EFAULT;
 		goto quit;
@@ -5067,8 +5067,9 @@ sppp_params(struct sppp *sp, u_long cmd, void *data)
 		goto quit;
 	}
 
-	switch (subcmd) {
-	case (u_long)SPPPIOGDEFS:
+	MPASS(subcmd == spr->cmd);
+	switch (spr->cmd) {
+	case (intptr_t)SPPPIOGDEFS:
 		if (cmd != SIOCGIFGENERIC) {
 			rv = EINVAL;
 			break;
@@ -5103,7 +5104,7 @@ sppp_params(struct sppp *sp, u_long cmd, void *data)
 		    sizeof(struct spppreq));
 		break;
 
-	case (u_long)SPPPIOSDEFS:
+	case (intptr_t)SPPPIOSDEFS:
 		if (cmd != SIOCSIFGENERIC) {
 			rv = EINVAL;
 			break;