Skip to content

Commit

Permalink
sppp: Fix getting wrong spppreq cmd from ioctl
Browse files Browse the repository at this point in the history
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 f9d8181, `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 f9d8181 `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:	f9d8181 Fixed yet more ioctl breakage due to the type of ...
Differential Revision:	https://reviews.freebsd.org/D47335
  • Loading branch information
gmshake committed Jan 14, 2025
1 parent 1639519 commit 08ec14f
Showing 1 changed file with 7 additions and 6 deletions.
13 changes: 7 additions & 6 deletions sys/net/if_spppsubr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 08ec14f

Please sign in to comment.