git: 08ec14fecf6a - stable/13 - sppp: Fix getting wrong spppreq cmd from ioctl
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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;