NetBSD Problem Report #44054
From www@NetBSD.org Sat Nov 6 11:21:40 2010
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
by www.NetBSD.org (Postfix) with ESMTP id D590063BAC6
for <gnats-bugs@gnats.NetBSD.org>; Sat, 6 Nov 2010 11:21:40 +0000 (UTC)
Message-Id: <20101106112140.6B38563BAB2@www.NetBSD.org>
Date: Sat, 6 Nov 2010 11:21:40 +0000 (UTC)
From: o.vd.linden@quicknet.nl
Reply-To: o.vd.linden@quicknet.nl
To: gnats-bugs@NetBSD.org
Subject: Stacksmashing in handling of ioctl OOSIO* parameter
X-Send-Pr-Version: www-1.0
>Number: 44054
>Category: kern
>Synopsis: Stacksmashing in handling of ioctl OOSIO* parameter
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: kern-bug-people
>State: open
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat Nov 06 11:25:00 +0000 2010
>Closed-Date:
>Last-Modified: Tue May 12 19:20:59 +0000 2015
>Originator: Onno van der Linden
>Release: NetBSD/i386 5.99.39
>Organization:
>Environment:
NetBSD sheep 5.99.39 NetBSD 5.99.39 (SHEEP) #35: Sat Nov 6 11:40:11 MET 2010 onno@sheep:/usr/src/sys/arch/i386/compile/SHEEP i386
>Description:
For ioctl parameters OOSIOCGIFADDR, OOSIOCGIFDSTADDR, OOSIOCGIFBRDADDR and OOSIOCGIFNETMASK a wrongly sized data buffer is passed on the stack to
ifreq_setaddr() where the call to memset() will smash the stack.
>How-To-Repeat:
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <netinet/in.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
struct oifreq {
char ifr_name[IFNAMSIZ]; /* if name, e.g. "en0" */
union {
struct sockaddr ifru_addr;
struct sockaddr ifru_dstaddr;
struct sockaddr ifru_broadaddr;
short ifru_flags;
int ifru_metric;
int ifru_mtu;
int ifru_dlt;
u_int ifru_value;
void * ifru_data;
struct {
uint32_t b_buflen;
void *b_buf;
} ifru_b;
} ifr_ifru;
};
#define OOSIOCGIFBRDADDR _IOWR('i', 18, struct oifreq)
main()
{
int fd;
struct oifreq ifreq;
struct sockaddr_in *sin;
memset(&ifreq,'\0',sizeof ifreq);
strcpy(ifreq.ifr_name,"wm0");
fd = socket(AF_INET, SOCK_DGRAM, 0);
if (fd < 0) {
perror("socket");
exit(1);
}
if (ioctl(fd, OOSIOCGIFBRDADDR, &ifreq) < 0) {
perror("OOSIOCGIFBRDADDR");
exit(1);
}
sin = (struct sockaddr_in *)&ifreq.ifr_broadaddr;
printf("broadcast: %s\n", inet_ntoa(sin->sin_addr));
close(fd);
}
>Fix:
compat_cvtcmd() doesn't translate the OOSIO parameter into its SIO counterpart,
there's some special handling in compat_ifioctl() already which could be used
to pass a rightly sized buffer, like the way the OSIO parameters are handled in ifioctl().
Another possible fix is to put a case statement in compat_cvtcmd() in place of the
ncmd assignment where the OOSIO parameters (and others ?) are handled as
seperate cases and in which the default case gets the ncmd assignment. But
that probably needs some additional changes to compat_ifioctl() too.
>Release-Note:
>Audit-Trail:
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/44054 CVS commit: src/sys/net
Date: Sat, 6 Nov 2010 13:17:14 -0400
Module Name: src
Committed By: christos
Date: Sat Nov 6 17:17:13 UTC 2010
Modified Files:
src/sys/net: if.c
Log Message:
PR/44054: Onno van der Linden: Stacksmashing in handling of ioctl OOSIO*
parameter.
To generate a diff of this commit:
cvs rdiff -u -r1.246 -r1.247 src/sys/net/if.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, kern-bug-people@netbsd.org,
gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc:
Subject: Re: kern/44054: Stacksmashing in handling of ioctl OOSIO* parameter
Date: Sat, 6 Nov 2010 13:22:27 -0400
On Nov 6, 11:25am, o.vd.linden@quicknet.nl (o.vd.linden@quicknet.nl) wrote:
-- Subject: kern/44054: Stacksmashing in handling of ioctl OOSIO* parameter
Fixed in the kernel, and here's the corrected test code for reference.
christos
#include <sys/socket.h>
#include <sys/ioctl.h>
#include <net/if.h>
#include <netinet/in.h>
#include <arpa/inet.h>
#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <err.h>
struct oifreq {
char ifr_name[IFNAMSIZ]; /* if name, e.g. "en0" */
union {
struct sockaddr ifru_addr;
struct sockaddr ifru_dstaddr;
struct sockaddr ifru_broadaddr;
short ifru_flags;
int ifru_metric;
int ifru_mtu;
int ifru_dlt;
u_int ifru_value;
void * ifru_data;
struct {
uint32_t b_buflen;
void *b_buf;
} ifru_b;
} ifr_ifru;
};
#define OOSIOCGIFBRDADDR _IOWR('i', 18, struct oifreq)
int
main(void)
{
int fd;
struct oifreq ifreq;
struct sockaddr_in *sin;
memset(&ifreq, '\0', sizeof ifreq);
strcpy(ifreq.ifr_name, "sk0");
fd = socket(AF_INET, SOCK_DGRAM, 0);
if (fd == -1)
err(1, "socket");
sin = (struct sockaddr_in *)&ifreq.ifr_broadaddr;
sin->sin_family = AF_INET;
sin->sin_len = sizeof(*sin);
if (ioctl(fd, OOSIOCGIFBRDADDR, &ifreq) == -1)
err(1, "OOSIOCGIFBRDADDR");
printf("broadcast: %s\n", inet_ntoa(sin->sin_addr));
close(fd);
return 0;
}
State-Changed-From-To: open->feedback
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Sun, 07 Nov 2010 00:16:11 +0000
State-Changed-Why:
christos committed a fix, does it work for you?
From: Onno van der Linden <o.vd.linden@quicknet.nl>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: kern/44054 (Stacksmashing in handling of ioctl OOSIO*
parameter)
Date: Sun, 7 Nov 2010 12:48:26 +0100
On Sun, Nov 07, 2010 at 12:16:12AM +0000, wiz@NetBSD.org wrote:
> Synopsis: Stacksmashing in handling of ioctl OOSIO* parameter
>
> State-Changed-From-To: open->feedback
> State-Changed-By: wiz@NetBSD.org
> State-Changed-When: Sun, 07 Nov 2010 00:16:11 +0000
> State-Changed-Why:
> christos committed a fix, does it work for you?
Works fine now.
Onno
From: "Antti Kantee" <pooka@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/44054 CVS commit: src/tests/net
Date: Sun, 7 Nov 2010 19:53:42 +0000
Module Name: src
Committed By: pooka
Date: Sun Nov 7 19:53:42 UTC 2010
Modified Files:
src/tests/net: Makefile
Added Files:
src/tests/net/if: Makefile t_compat.c
Log Message:
convert program in PR kern/44054 to an atf test case
To generate a diff of this commit:
cvs rdiff -u -r1.6 -r1.7 src/tests/net/Makefile
cvs rdiff -u -r0 -r1.1 src/tests/net/if/Makefile src/tests/net/if/t_compat.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: feedback->closed
State-Changed-By: wiz@NetBSD.org
State-Changed-When: Mon, 08 Nov 2010 00:20:02 +0000
State-Changed-Why:
Confirmed fixed, and test case added.
Thanks!
From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/44054 CVS commit: [netbsd-5] src/sys
Date: Wed, 16 Feb 2011 20:37:47 +0000
Module Name: src
Committed By: bouyer
Date: Wed Feb 16 20:37:47 UTC 2011
Modified Files:
src/sys/compat/common [netbsd-5]: uipc_syscalls_43.c
src/sys/net [netbsd-5]: if.c
Log Message:
Pull up following revision(s) (requested by chs in ticket #1541):
sys/compat/common/if_43.c: revision 1.3 via patch
sys/net/if.c: revision 1.247 via patch
PR/44054: Onno van der Linden: Stacksmashing in handling of ioctl OOSIO*
parameter.
can't map the old and the new SIO calls the way we did before because the
numbers have changed. Instead provide a switch. Keep the old code there,
to handle cases we did not handle in the first switch, but this is a hack
and should be removed.
To generate a diff of this commit:
cvs rdiff -u -r1.43 -r1.43.10.1 src/sys/compat/common/uipc_syscalls_43.c
cvs rdiff -u -r1.230.4.3 -r1.230.4.4 src/sys/net/if.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Stephen Borrill" <sborrill@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/44054 CVS commit: [netbsd-5] src/sys/net
Date: Thu, 5 Jan 2012 09:47:39 +0000
Module Name: src
Committed By: sborrill
Date: Thu Jan 5 09:47:39 UTC 2012
Modified Files:
src/sys/net [netbsd-5]: if.c
Log Message:
Pull up the following revisions(s) (requested by obache in ticket #1708):
sys/net/if.c: revision 1.246
PR/44030: ifreqn2o gets called with the parameters the wrong way around.
Reverts fix for PR 42585 (ticket #1416) as the root cause of the crash is
addressed by PR 44054 (pullup #1541).
To generate a diff of this commit:
cvs rdiff -u -r1.230.4.5 -r1.230.4.6 src/sys/net/if.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: closed->open
State-Changed-By: martin@NetBSD.org
State-Changed-When: Tue, 12 May 2015 19:20:59 +0000
State-Changed-Why:
There is still a problem in kernels not using the compat code
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.