linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel
@ 2003-05-01  4:39 Junfeng Yang
  2003-05-01  4:55 ` [CHECKER] 2 potential passing kernel-pointer into copy_*_user errors Junfeng Yang
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-01  4:39 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: mc

Hi,

This is a resend (the previous report was ignored, however I feel that
these bugs could be severe).

Here are 5 bugs in 2.5.63 where user pointers are passed into memcpy or
simple_strtoul without verifications. These bugs appear functions assigned
to struct proc_dir_entry.write_proc, where a malicious user can call
write_proc on a arbitrary pointer pointing to any sensitive kernel data,
then call the corresponding read_proc to get back the data.

Please confirm or clarify. Thanks!

-Junfeng

---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs

/home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain:
ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_gain((tainted
1))]

static int vicam_write_proc_gain(struct file *file, const char *buffer,
				unsigned long count, void *data)
{
	struct vicam_camera *cam = (struct vicam_camera *)data;


Error --->
	cam->gain = simple_strtoul(buffer, NULL, 10);

	return count;
}
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs

/home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1107:vicam_write_proc_shutter:
ERROR:TAINTED:1107:1107: passing tainted ptr 'buffer' to simple_strtoul
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_shutter((tainted
1))]

static int vicam_write_proc_shutter(struct file *file, const char *buffer,
				unsigned long count, void *data)
{
	struct vicam_camera *cam = (struct vicam_camera *)data;


Error --->
	cam->shutter_speed = simple_strtoul(buffer, NULL, 10);

	return count;
}
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc

/home/junfeng/linux-2.5.63/drivers/media/video/zoran_procfs.c:122:zoran_write_proc:
ERROR:TAINTED:122:122: passing tainted ptr 'buffer' to __memcpy
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:zoran_write_proc((tainted
1))]

	string = sp = vmalloc(count + 1);
	if (!string) {
		printk(KERN_ERR "%s: write_proc: can not allocate
memory\n", zr->name);
		return -ENOMEM;
	}

Error --->
	memcpy(string, buffer, count);
	string[count] = 0;
	DEBUG2(printk(KERN_INFO "%s: write_proc: name=%s count=%lu
data=%x\n", zr->name, file->f_dentry->d_name.name, count, (int) data));
	ldelim = " \t\n";
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc

/home/junfeng/linux-2.5.63/drivers/pnp/pnpbios/proc.c:190:proc_write_node:
ERROR:TAINTED:190:190: passing tainted ptr 'buf' to __memcpy [Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:proc_write_node((tainted
1))]

	if (!node) return -ENOMEM;
	if ( pnp_bios_get_dev_node(&nodenum, boot, node) )
		return -EIO;
	if (count != node->size - sizeof(struct pnp_bios_node))
		return -EINVAL;

Error --->
	memcpy(node->data, buf, count);
	if (pnp_bios_set_dev_node(node->handle, boot, node) != 0)
	    return -EINVAL;
	kfree(node);
---------------------------------------------------------
[BUG] proc_dir_entry.write_proc can take tainted inputs.
av7110_ir_write_proc is assigned to proc_dir_entry.write_proc

/home/junfeng/linux-2.5.63/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc:
ERROR:TAINTED:116:116: passing tainted ptr 'buffer' to __constant_memcpy
[Callstack:
/home/junfeng/linux-2.5.63/net/core/pktgen.c:991:av7110_ir_write_proc((tainted
1))]

	u32 ir_config;

	if (count < 4 + 256 * sizeof(u16))
		return -EINVAL;


Error --->
	memcpy (&ir_config, buffer, 4);
	memcpy (&key_map, buffer + 4, 256 * sizeof(u16));

	av7110_setup_irc_config (NULL, ir_config);




^ permalink raw reply	[flat|nested] 15+ messages in thread

* [CHECKER] 2 potential passing kernel-pointer into copy_*_user errors
  2003-05-01  4:39 [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
@ 2003-05-01  4:55 ` Junfeng Yang
  2003-05-01  5:45 ` [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-01  4:55 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: mc


Hi,

Below are 2 more warnings where kernel pointer is passed into *_do_ioctl
(these functions are passed into video_usercopy). Please note that our
checker flags the dereferences as errors, where the actually errors should
be the copy_*_user calls.

Thanks a lot!

-Junfeng

---------------------------------------------------------
[BUG] pass kernel pointer into copy_*_user. bug is in VIDIOCGTUNER. Should
not call copy_to_user on arg since arg is already in kernel space.

/home/junfeng/linux-2.5.63/drivers/media/radio/radio-cadet.c:397:cadet_do_ioctl:
ERROR:TAINTED:397:397: dereferencing tainted ptr 'v' [Callstack: ]

	{
		case VIDIOCGCAP:
		{
			struct video_capability *v = arg;
			memset(v,0,sizeof(*v));

Error --->
			v->type=VID_TYPE_TUNER;
			v->channels=2;
			v->audios=1;
			strcpy(v->name, "ADS Cadet");
---------------------------------------------------------
[BUG] pass kernel pointer into copy_*_user. should not call copy_to_user
on case VIDIOCGCHAN

/home/junfeng/linux-2.5.63/drivers/media/video/bw-qcam.c:763:qcam_do_ioctl:
ERROR:TAINTED:763:763: dereferencing tainted ptr 'p' [Callstack: ]

			return 0;
		}
		case VIDIOCGPICT:
		{
			struct video_picture *p = arg;

Error --->
			p->colour=0x8000;
			p->hue=0x8000;
			p->brightness=qcam->brightness<<8;
			p->contrast=qcam->contrast<<8;



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel
  2003-05-01  4:39 [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
  2003-05-01  4:55 ` [CHECKER] 2 potential passing kernel-pointer into copy_*_user errors Junfeng Yang
@ 2003-05-01  5:45 ` Junfeng Yang
  2003-05-01 12:54 ` Michael Hunold
  2003-05-01 20:52 ` Greg KH
  3 siblings, 0 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-01  5:45 UTC (permalink / raw)
  To: Linux Kernel Mailing List


Also, please note the word "Callstack" in the error reports doesn't really
mean an actual call here. It means "functions are linked together if they
are assigned to the same structure field".

for example, the following error report is catched in this way:

net/core/pktgen.c:proc_write treats its second arguemtn as tainted.
we take it as a good code example, and require that all the other
functions that are assigned to proc_dir_entry.write_proc must treat its
second argument as tainted.

sorry for the confusions.

> ---------------------------------------------------------
> [BUG] proc_dir_entry.write_proc can take tainted inputs
>
> /home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain:
> ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul
> [Callstack:
> /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_gain((tainted
> 1))]
>
> static int vicam_write_proc_gain(struct file *file, const char *buffer,
> 				unsigned long count, void *data)
> {
> 	struct vicam_camera *cam = (struct vicam_camera *)data;
>
>
> Error --->
> 	cam->gain = simple_strtoul(buffer, NULL, 10);
>
> 	return count;
> }


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel
  2003-05-01  4:39 [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
  2003-05-01  4:55 ` [CHECKER] 2 potential passing kernel-pointer into copy_*_user errors Junfeng Yang
  2003-05-01  5:45 ` [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
@ 2003-05-01 12:54 ` Michael Hunold
  2003-05-01 20:07   ` Junfeng Yang
  2003-05-01 20:52 ` Greg KH
  3 siblings, 1 reply; 15+ messages in thread
From: Michael Hunold @ 2003-05-01 12:54 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: Linux Kernel Mailing List, mc

Hello Junfeng,

> This is a resend (the previous report was ignored, however I feel that
> these bugs could be severe).

> Please confirm or clarify. Thanks!

> [BUG] proc_dir_entry.write_proc can take tainted inputs.
> av7110_ir_write_proc is assigned to proc_dir_entry.write_proc
> 
> /home/junfeng/linux-2.5.63/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc:
> ERROR:TAINTED:116:116: passing tainted ptr 'buffer' to __constant_memcpy
> [Callstack:
> /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:av7110_ir_write_proc((tainted
> 1))]

Confirmed. I'll post a patch when I'm back at work again on Monday.

CU
Michael.


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel
  2003-05-01 12:54 ` Michael Hunold
@ 2003-05-01 20:07   ` Junfeng Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-01 20:07 UTC (permalink / raw)
  To: Michael Hunold; +Cc: Linux Kernel Mailing List, mc


Thanks a lot for the feedback and the incoming patch!

-Junfeng

On Thu, 1 May 2003, Michael Hunold wrote:

> Hello Junfeng,
>
> > This is a resend (the previous report was ignored, however I feel that
> > these bugs could be severe).
>
> > Please confirm or clarify. Thanks!
>
> > [BUG] proc_dir_entry.write_proc can take tainted inputs.
> > av7110_ir_write_proc is assigned to proc_dir_entry.write_proc
> >
> > /home/junfeng/linux-2.5.63/drivers/media/dvb/av7110/av7110_ir.c:116:av7110_ir_write_proc:
> > ERROR:TAINTED:116:116: passing tainted ptr 'buffer' to __constant_memcpy
> > [Callstack:
> > /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:av7110_ir_write_proc((tainted
> > 1))]
>
> Confirmed. I'll post a patch when I'm back at work again on Monday.
>
> CU
> Michael.
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel
  2003-05-01  4:39 [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
                   ` (2 preceding siblings ...)
  2003-05-01 12:54 ` Michael Hunold
@ 2003-05-01 20:52 ` Greg KH
  2003-05-01 20:53   ` Junfeng Yang
  3 siblings, 1 reply; 15+ messages in thread
From: Greg KH @ 2003-05-01 20:52 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: Linux Kernel Mailing List, mc

On Wed, Apr 30, 2003 at 09:39:18PM -0700, Junfeng Yang wrote:
> ---------------------------------------------------------
> [BUG] proc_dir_entry.write_proc can take tainted inputs
> 
> /home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain:
> ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul
> [Callstack:
> /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_gain((tainted
> 1))]
> 
> static int vicam_write_proc_gain(struct file *file, const char *buffer,
> 				unsigned long count, void *data)
> {
> 	struct vicam_camera *cam = (struct vicam_camera *)data;
> 
> 
> Error --->
> 	cam->gain = simple_strtoul(buffer, NULL, 10);

Real bug, I'll fix this.

> ---------------------------------------------------------
> [BUG] proc_dir_entry.write_proc can take tainted inputs
> 
> /home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1107:vicam_write_proc_shutter:
> ERROR:TAINTED:1107:1107: passing tainted ptr 'buffer' to simple_strtoul
> [Callstack:
> /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_shutter((tainted
> 1))]
> 
> static int vicam_write_proc_shutter(struct file *file, const char *buffer,
> 				unsigned long count, void *data)
> {
> 	struct vicam_camera *cam = (struct vicam_camera *)data;
> 
> 
> Error --->
> 	cam->shutter_speed = simple_strtoul(buffer, NULL, 10);

Again, real bug, I'll fix it.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel
  2003-05-01 20:52 ` Greg KH
@ 2003-05-01 20:53   ` Junfeng Yang
  2003-05-02  6:43     ` [CHECKER] 4 potential user-pointer errors Junfeng Yang
  2003-05-12  6:29     ` [CHECKER] 1 potential derefence of user-pointer without verification error Junfeng Yang
  0 siblings, 2 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-01 20:53 UTC (permalink / raw)
  To: Greg KH; +Cc: Linux Kernel Mailing List, mc


Thanks!

On Thu, 1 May 2003, Greg KH wrote:

> On Wed, Apr 30, 2003 at 09:39:18PM -0700, Junfeng Yang wrote:
> > ---------------------------------------------------------
> > [BUG] proc_dir_entry.write_proc can take tainted inputs
> >
> > /home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1117:vicam_write_proc_gain:
> > ERROR:TAINTED:1117:1117: passing tainted ptr 'buffer' to simple_strtoul
> > [Callstack:
> > /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_gain((tainted
> > 1))]
> >
> > static int vicam_write_proc_gain(struct file *file, const char *buffer,
> > 				unsigned long count, void *data)
> > {
> > 	struct vicam_camera *cam = (struct vicam_camera *)data;
> >
> >
> > Error --->
> > 	cam->gain = simple_strtoul(buffer, NULL, 10);
>
> Real bug, I'll fix this.
>
> > ---------------------------------------------------------
> > [BUG] proc_dir_entry.write_proc can take tainted inputs
> >
> > /home/junfeng/linux-2.5.63/drivers/usb/media/vicam.c:1107:vicam_write_proc_shutter:
> > ERROR:TAINTED:1107:1107: passing tainted ptr 'buffer' to simple_strtoul
> > [Callstack:
> > /home/junfeng/linux-2.5.63/net/core/pktgen.c:991:vicam_write_proc_shutter((tainted
> > 1))]
> >
> > static int vicam_write_proc_shutter(struct file *file, const char *buffer,
> > 				unsigned long count, void *data)
> > {
> > 	struct vicam_camera *cam = (struct vicam_camera *)data;
> >
> >
> > Error --->
> > 	cam->shutter_speed = simple_strtoul(buffer, NULL, 10);
>
> Again, real bug, I'll fix it.
>
> thanks,
>
> greg k-h
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [CHECKER] 4 potential user-pointer errors
  2003-05-01 20:53   ` Junfeng Yang
@ 2003-05-02  6:43     ` Junfeng Yang
  2003-05-09 21:44       ` [CHECKER] Clarifications needed on a user-pointer false alarm in kernel/kmod.c Junfeng Yang
  2003-05-12  6:29     ` [CHECKER] 1 potential derefence of user-pointer without verification error Junfeng Yang
  1 sibling, 1 reply; 15+ messages in thread
From: Junfeng Yang @ 2003-05-02  6:43 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: mc


Hi,

Enclosed are 4 more potential bugs where user pointer is dereferenced.
Please note the word "Callstack" in the bug report doesn't always mean a
call. We link functions together if they are assigned to the same struct
field.

Please confirm or clarify. Thanks!

-Junfeng

---------------------------------------------------------
[BUG] at least bad programming practice. file_operations.write ->
cm_write -> trans_ac3. write can take tainted. write can take tainted
inputs. the pointer is vefied in cm_write

/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:593:trans_ac3:
ERROR:TAINTED:593:593: dereferencing tainted ptr 'src' [Callstack:
/home/junfeng/linux-2.5.63/fs/read_write.c:307:vfs_write((tainted
1)(tainted 2)) ->
/home/junfeng/linux-2.5.63/fs/read_write.c:241:cm_write((tainted
1)(tainted 2)) ->
/home/junfeng/linux-2.5.63/sound/oss/cmpci.c:1662:trans_ac3((tainted
2))]

	unsigned long data;
	unsigned long *dst = (unsigned long *) dest;
	unsigned short *src = (unsigned short *)source;

	do {

Error --->
		data = (unsigned long) *src++;
		data <<= 12;			// ok for 16-bit data
		if (s->spdif_counter == 2 || s->spdif_counter == 3)
			data |= 0x40000000;	// indicate AC-3 raw
data
---------------------------------------------------------
[BUG] at least bad programming practice. file_opetations.ioctl ->
aac_cfg_ioctl -> aac_do_ioctl -> close_getadapter_fib ->
aac_close_fib_context. All other functions called by aac_do_ioctl assume
arg is a user pointer. It is unknown what will happen.

/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/commctrl.c:277:aac_close_fib_context:
ERROR:TAINTED:277:277: dereferencing tainted ptr 'fibctx' [Callstack:
/home/junfeng/linux-2.5.63/drivers/scsi/sg.c:1002:aac_cfg_ioctl((tainted
3)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/linit.c:671:aac_do_ioctl((tainted
2)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/commctrl.c:421:close_getadapter_fib((tainted
1)) ->
/home/junfeng/linux-2.5.63/drivers/scsi/aacraid/commctrl.c:348:aac_close_fib_context((tainted
1))]

	while (!list_empty(&fibctx->fibs)) {
		struct list_head * entry;
		/*
		 *	Pull the next fib from the fibs
		 */

Error --->
		entry = fibctx->fibs.next;
		list_del(entry);
		fib = list_entry(entry, struct hw_fib, header.FibLinks);
		fibctx->count--;
---------------------------------------------------------
[BUG] at least bad programming practice. zoran_read and vbi_read are
both assigned to video_device.read, while zoran_read assumes "buf" is
tainted. The pointer is verified by access_ok

/home/junfeng/linux-2.5.63/drivers/media/video/zr36120.c:1698:vbi_read:
ERROR:TAINTED:1698:1698: dereferencing tainted ptr 'optr' [Callstack:
/home/junfeng/linux-2.5.63/drivers/media/video/zr36120.c:934:vbi_read((tainted
1))]

		{
			/* copy to doubled data to userland */
			for (x=0; optr+1<eptr && x<-done->w; x++)
			{
				unsigned char a = iptr[x*2];

Error --->
				*optr++ = a;
				*optr++ = a;
			}
			/* and clear the rest of the line */
---------------------------------------------------------
[BUG] at least bad programming practice. calls access_ok on
qv.packet_sizes, then passed qv.packet_sizes into
initialize_dma_it_prg_var_packet_queue.

/home/junfeng/linux-2.5.63/drivers/ieee1394/video1394.c:668:initialize_dma_it_prg_var_packet_queue:
ERROR:TAINTED:668:668: dereferencing tainted ptr 'packet_sizes + i * 4'
[Callstack:
/home/junfeng/linux-2.5.63/drivers/ieee1394/video1394.c:1047:initialize_dma_it_prg_var_packet_queue((tainted
2))]

	for (i = 0; i < d->nb_cmd; i++) {
		unsigned int size;
		if (packet_sizes[i] > d->packet_size) {
			size = d->packet_size;
		} else {

Error --->
			size = packet_sizes[i];
		}
		it_prg[i].data[1] = cpu_to_le32(size << 16);
		it_prg[i].end.control = cpu_to_le32(DMA_CTL_OUTPUT_LAST
| DMA_CTL_BRANCH);


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [CHECKER] Clarifications needed on a user-pointer false alarm in kernel/kmod.c
  2003-05-02  6:43     ` [CHECKER] 4 potential user-pointer errors Junfeng Yang
@ 2003-05-09 21:44       ` Junfeng Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-09 21:44 UTC (permalink / raw)
  To: Linux Kernel Mailing List


Hi,

I got the following false alarm in kernel/kmod.c.

the call chain is sys_wait4 (_, &sub_info->retval) -> wait_task_zombie (_,
_, stat_addr, _) -> put_user (_, stat_addr), which means &sub_info->retval
will be passed into put_user. From the calling context, sub_info should be
in kernel space, so &sub_info->retval should be in kernel space as well.
The explanation for this false alarm could be that the call chain wasn't
realistic, but I'm not sure. Can you guys please help me on that?

/home/junfeng/linux-tainted/kernel/kmod.c:185:wait_for_helper:
ERROR:TAINTED:185:185: dereferencing tainted ptr 'sub_info' [Callstack: ]
  if (pid < 0)
            sub_info->retval = pid;
 else
            sys_wait4(pid, (unsigned int *)&sub_info->retval, 0, NULL);


Error --->
   complete(sub_info->complete);
   return 0;
}



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [CHECKER] 1 potential derefence of user-pointer without verification error
  2003-05-01 20:53   ` Junfeng Yang
  2003-05-02  6:43     ` [CHECKER] 4 potential user-pointer errors Junfeng Yang
@ 2003-05-12  6:29     ` Junfeng Yang
  2003-05-12  6:44       ` Junfeng Yang
  2003-05-12 21:19       ` [CHECKER] One more potential user-pointer error Junfeng Yang
  1 sibling, 2 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-12  6:29 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: mc


Hi,

Below is a warning found in pcmcia/ds.c, where user pointer is
dereferenced without check. Please confirm or clarify, Thanks!

-Junfeng

---------------------------------------------------------

[BUG] buf is tainted implies buf.win_info.handle is tainted.
pcmcia_get_mem_page dereferences its first parameter

/home/junfeng/linux-tainted/drivers/pcmcia/ds.c:814:ds_ioctl:
ERROR:TAINTED: 814:814:deref tainted component 'buf.win_info.handle'
[struct=win_info_t.handle] [type=call]

	break;
    case DS_GET_NEXT_WINDOW:
	ret = pcmcia_get_next_window(&buf.win_info.handle,
&buf.win_info.window);
	break;
    case DS_GET_MEM_PAGE:

Error --->
	ret = pcmcia_get_mem_page(buf.win_info.handle,
			   &buf.win_info.map);
	break;
    case DS_REPLACE_CIS:



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 1 potential derefence of user-pointer without verification error
  2003-05-12  6:29     ` [CHECKER] 1 potential derefence of user-pointer without verification error Junfeng Yang
@ 2003-05-12  6:44       ` Junfeng Yang
  2003-05-15 20:40         ` [CHECKER] 2 potential out-of-bound user-pointer errors in fs/readdir.c Junfeng Yang
  2003-05-12 21:19       ` [CHECKER] One more potential user-pointer error Junfeng Yang
  1 sibling, 1 reply; 15+ messages in thread
From: Junfeng Yang @ 2003-05-12  6:44 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: mc


here is a detailed explanation in case the warnning itself isn't clear:

1. ds_ioctl is assigned to file_operantions.ioctl
so its argument 'arg' is tainted. verify_area are
also called on 'arg', which confirms.

2. copy_from_user (&buf, arg, _) copies in the content of arg

3. buf.win_info.handle is thus a user provided pointer.

4. pcmcia_get_mem_page dereferences its first parameter, in this case
buf.win_info.handle

-Junfeng

On Sun, 11 May 2003, Junfeng Yang wrote:

>
> Hi,
>
> Below is a warning found in pcmcia/ds.c, where user pointer is
> dereferenced without check. Please confirm or clarify, Thanks!
>
> -Junfeng
>
> ---------------------------------------------------------
>
> [BUG] buf is tainted implies buf.win_info.handle is tainted.
> pcmcia_get_mem_page dereferences its first parameter
>
> /home/junfeng/linux-tainted/drivers/pcmcia/ds.c:814:ds_ioctl:
> ERROR:TAINTED: 814:814:deref tainted component 'buf.win_info.handle'
> [struct=win_info_t.handle] [type=call]
>
> 	break;
>     case DS_GET_NEXT_WINDOW:
> 	ret = pcmcia_get_next_window(&buf.win_info.handle,
> &buf.win_info.window);
> 	break;
>     case DS_GET_MEM_PAGE:
>
> Error --->
> 	ret = pcmcia_get_mem_page(buf.win_info.handle,
> 			   &buf.win_info.map);
> 	break;
>     case DS_REPLACE_CIS:
>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [CHECKER] One more potential user-pointer error
  2003-05-12  6:29     ` [CHECKER] 1 potential derefence of user-pointer without verification error Junfeng Yang
  2003-05-12  6:44       ` Junfeng Yang
@ 2003-05-12 21:19       ` Junfeng Yang
  1 sibling, 0 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-12 21:19 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: faith, ajoshi, mc


Hi,

Enclosed is one warning in drivers/char/drm/radeon_state.c  I've attached
a detaillsed explanation. If it is not clear, please mail me.

As always, please confirm or clarify. Thanks!

-Junfeng

---------------------------------------------------------

[BUG] function radeon_cp_dispatch_indices calls
DRM_COPY_FROM_USER_UNCHECKED on parameter 'dev_priv->sarea_priv->boxes',
implies it is a user space pointer. dev_piv->sarea_priv has type
'drm_radeon_sarea_t', where field 'drm_radeon_sarea_t.boxes' is declared
as an array. so dev_priv->sarea_priv->boxes is equivalent to
dev_priv->sarea_priv + offset of field 'boxes'. since dev_priv->sarea_priv
+ offset_of_'boxes' is tainted, dev_priv->sarea_priv is also a user-space
pointer. this pointer is deref'd several times.

/home/junfeng/linux-tainted/drivers/char/drm/radeon_state.c:1554:radeon_cp_indices:
ERROR:TAINTED:1554:1554: dereferencing tainted ptr 'dev_priv->sarea_priv'
[Callstack: ]

	prim.prim = elts.prim;
	prim.offset = 0;	/* offset from start of dma buffers */
	prim.numverts = RADEON_MAX_VB_VERTS; /* duh */
	prim.vc_format = dev_priv->sarea_priv->vc_format;


Error --->
	radeon_cp_dispatch_indices( dev, buf, &prim,
				   dev_priv->sarea_priv->boxes,
				   dev_priv->sarea_priv->nbox );
	if (elts.discard) {
---------------------------------------------------------
[BUG]

/home/junfeng/linux-tainted/drivers/char/drm/radeon_state.c:1773:radeon_cp_vertex2:
ERROR:TAINTED:1773:1773: dereferencing tainted ptr 'sarea_priv'
[Callstack: ]


		if ( prim.prim & RADEON_PRIM_WALK_IND ) {
			tclprim.offset = prim.numverts * 64;
			tclprim.numverts = RADEON_MAX_VB_VERTS; /* duh */


Error --->
			radeon_cp_dispatch_indices( dev, buf, &tclprim,
						    sarea_priv->boxes,
						    sarea_priv->nbox);
		} else {
---------------------------------------------------------
[BUG]

/home/junfeng/linux-tainted/drivers/char/drm/radeon_state.c:1454:radeon_cp_vertex:
ERROR:TAINTED:1454:1454: dereferencing tainted ptr 'dev_priv->sarea_priv'
[Callstack: ]

		prim.finish = vertex.count; /* unused */
		prim.prim = vertex.prim;
		prim.numverts = vertex.count;
		prim.vc_format = dev_priv->sarea_priv->vc_format;


Error --->
		radeon_cp_dispatch_vertex( dev, buf, &prim,
					   dev_priv->sarea_priv->boxes,
					   dev_priv->sarea_priv->nbox );
	}



^ permalink raw reply	[flat|nested] 15+ messages in thread

* [CHECKER] 2 potential out-of-bound user-pointer errors in fs/readdir.c
  2003-05-12  6:44       ` Junfeng Yang
@ 2003-05-15 20:40         ` Junfeng Yang
  2003-05-15 22:03           ` Andrew Morton
  0 siblings, 1 reply; 15+ messages in thread
From: Junfeng Yang @ 2003-05-15 20:40 UTC (permalink / raw)
  To: Linux Kernel Mailing List; +Cc: mc


Hi,

Enclosed are two warnings found in fs/readdir.c, where user provided
pointers are accessed out of 'verified' bounds.

The warnings are found by: first, whenenver we see calls to verify_area,
access_ok and all the no-underscore versions of *_user functions, we
remember the verified bounds. when a user-pointer is accessed thru
__*_user functions, we check if the verified bound is big enough.

Please confirm or clarify. Thanks!

-Junfeng


---------------------------------------------------------

[BUG] copy_to_user verifies that [0, namelen) of dirent->d_name is okay to
write, but the following __put_user accesses dirent->d_name + namelen

/home/junfeng/linux-2.5.69/fs/readdir.c:239:filldir64:
ERROR:BUFFER:239:239:memory operation error (len < off + n) (__put_user(0,
(void*)(char*)&dirent->d_name + (char*)(long unsigned
int)namlen):(void*)(char*)&dirent->d_name + (char*)(long unsigned
int)namlen, len = sym_8, off = sym_8 + 0, n = 1, min (off + n - len = 1)

		goto efault;
	if (__put_user(d_type, &dirent->d_type))
		goto efault;
	if (copy_to_user(dirent->d_name, name, namlen))
		goto efault;

Error --->
	if (__put_user(0, dirent->d_name + namlen))
		goto efault;
	((char *) dirent) += reclen;
	buf->current_dir = dirent;
---------------------------------------------------------

[BUG] copy_to_user verifies that [0, namelen) of dirent->d_name is okay to
write, but the following __put_user accesses dirent->d_name + namelen

/home/junfeng/linux-2.5.69/fs/readdir.c:154:filldir:
ERROR:BUFFER:154:154:memory operation error (len < off + n) (__put_user(0,
(void*)(char*)&dirent->d_name + (char*)(long unsigned
int)namlen):(void*)(char*)&dirent->d_name + (char*)(long unsigned
int)namlen, len = sym_8, off = sym_8 + 0, n = 1, min (off + n - len = 1)

		goto efault;
	if (__put_user(reclen, &dirent->d_reclen))
		goto efault;
	if (copy_to_user(dirent->d_name, name, namlen))
		goto efault;

Error --->
	if (__put_user(0, dirent->d_name + namlen))
		goto efault;
	((char *) dirent) += reclen;
	buf->current_dir = dirent;



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 2 potential out-of-bound user-pointer errors in fs/readdir.c
  2003-05-15 20:40         ` [CHECKER] 2 potential out-of-bound user-pointer errors in fs/readdir.c Junfeng Yang
@ 2003-05-15 22:03           ` Andrew Morton
  2003-05-16  0:29             ` Junfeng Yang
  0 siblings, 1 reply; 15+ messages in thread
From: Andrew Morton @ 2003-05-15 22:03 UTC (permalink / raw)
  To: Junfeng Yang; +Cc: linux-kernel, mc

Junfeng Yang <yjf@stanford.edu> wrote:
>
> 
> Hi,
> 
> Enclosed are two warnings found in fs/readdir.c, where user provided
> pointers are accessed out of 'verified' bounds.
> 
> The warnings are found by: first, whenenver we see calls to verify_area,
> access_ok and all the no-underscore versions of *_user functions, we
> remember the verified bounds. when a user-pointer is accessed thru
> __*_user functions, we check if the verified bound is big enough.
> 
> Please confirm or clarify. Thanks!

The code as-is appears to be OK.  Note how sys_getdents64() will run
access_ok() against the entire user buffer up-front.  Then the start/len of
that verified area is copied into the getdents_callback64 and that is
propagated down to filldir64().

And filldir64() looks like it correctly remains within the bounds of the
start/len.

I guess that copy_to_user() should be __copy_to_user().



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [CHECKER] 2 potential out-of-bound user-pointer errors in fs/readdir.c
  2003-05-15 22:03           ` Andrew Morton
@ 2003-05-16  0:29             ` Junfeng Yang
  0 siblings, 0 replies; 15+ messages in thread
From: Junfeng Yang @ 2003-05-16  0:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel


Thanks for the clarifications! so these are not real security bugs, but
redundant checks.

-Junfeng

On Thu, 15 May 2003, Andrew Morton wrote:

> Junfeng Yang <yjf@stanford.edu> wrote:
> >
> >
> > Hi,
> >
> > Enclosed are two warnings found in fs/readdir.c, where user provided
> > pointers are accessed out of 'verified' bounds.
> >
> > The warnings are found by: first, whenenver we see calls to verify_area,
> > access_ok and all the no-underscore versions of *_user functions, we
> > remember the verified bounds. when a user-pointer is accessed thru
> > __*_user functions, we check if the verified bound is big enough.
> >
> > Please confirm or clarify. Thanks!
>
> The code as-is appears to be OK.  Note how sys_getdents64() will run
> access_ok() against the entire user buffer up-front.  Then the start/len of
> that verified area is copied into the getdents_callback64 and that is
> propagated down to filldir64().
>
> And filldir64() looks like it correctly remains within the bounds of the
> start/len.
>
> I guess that copy_to_user() should be __copy_to_user().
>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2003-05-16  0:16 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-05-01  4:39 [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
2003-05-01  4:55 ` [CHECKER] 2 potential passing kernel-pointer into copy_*_user errors Junfeng Yang
2003-05-01  5:45 ` [CHECKER] 5 potential user-pointer errors that allow arbitrary reads from kernel Junfeng Yang
2003-05-01 12:54 ` Michael Hunold
2003-05-01 20:07   ` Junfeng Yang
2003-05-01 20:52 ` Greg KH
2003-05-01 20:53   ` Junfeng Yang
2003-05-02  6:43     ` [CHECKER] 4 potential user-pointer errors Junfeng Yang
2003-05-09 21:44       ` [CHECKER] Clarifications needed on a user-pointer false alarm in kernel/kmod.c Junfeng Yang
2003-05-12  6:29     ` [CHECKER] 1 potential derefence of user-pointer without verification error Junfeng Yang
2003-05-12  6:44       ` Junfeng Yang
2003-05-15 20:40         ` [CHECKER] 2 potential out-of-bound user-pointer errors in fs/readdir.c Junfeng Yang
2003-05-15 22:03           ` Andrew Morton
2003-05-16  0:29             ` Junfeng Yang
2003-05-12 21:19       ` [CHECKER] One more potential user-pointer error Junfeng Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).