The branch stable/13 has been updated by zlei:
URL:
https://cgit.FreeBSD.org/src/commit/?id=08ec14fecf6a93c0321c31ba1f0b04db6b888f16
commit 08ec14fecf6a93c0321c31ba1f0b04db6b888f16
Author: Zhenlei Huang <z...@freebsd.org>
AuthorDate: 2025-01-14 10:56:49 +0000
Commit: Zhenlei Huang <z...@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;
