Misc 2.5 Fixes: Summary

Misc 2.5 Fixes: Summary

[Dipankar Sarma]

This set of patches fix various problems mostly related to
copy/user problems and leaks. I worked based on Alan's list and
either forward ported from 2.4 whatever was available or fixed
by code inspection. They compile, but haven't really been tested.
They are diffed against 2.5.70.

Thanks
Dipankar

Re: Misc 2.5 Fixes: Summary

[Joshua Kwan]

[-- Attachment #1: Type: text/plain, Size: 266 bytes --]

On Tue, Jun 10, 2003 at 03:35:27PM +0530, Dipankar Sarma wrote:
> This set of patches fix various problems mostly related to
> copy/user problems and leaks. I worked based on Alan's list and

What set of patches?

-Josh

-- 
New PGP public key: 0x27AFC3EE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Misc 2.5 Fixes: arcnet-oops-fix

[Dipankar Sarma]

Fix arcnet oopses with raw socket


 drivers/net/arcnet/arc-rawmode.c |    9 ++++-----
 drivers/net/arcnet/arcnet.c      |   15 ++++++++-------
 drivers/net/arcnet/rfc1051.c     |    9 ++++-----
 drivers/net/arcnet/rfc1201.c     |    9 ++++-----
 include/linux/arcdevice.h        |    4 ++--
 5 files changed, 22 insertions(+), 24 deletions(-)

diff -puN drivers/net/arcnet/arcnet.c~arcnet-oops-fix drivers/net/arcnet/arcnet.c
--- linux-2.5.70-ds/drivers/net/arcnet/arcnet.c~arcnet-oops-fix	2003-06-08 02:09:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/net/arcnet/arcnet.c	2003-06-08 02:09:38.000000000 +0530
@@ -57,8 +57,8 @@
 /* "do nothing" functions for protocol drivers */
 static void null_rx(struct net_device *dev, int bufnum,
 		    struct archdr *pkthdr, int length);
-static int null_build_header(struct sk_buff *skb, unsigned short type,
-			     uint8_t daddr);
+static int null_build_header(struct sk_buff *skb, struct net_device *dev,
+			     unsigned short type, uint8_t daddr);
 static int null_prepare_tx(struct net_device *dev, struct archdr *pkt,
 			   int length, int bufnum);
 
@@ -479,7 +479,7 @@ static int arcnet_header(struct sk_buff 
 		       arc_bcast_proto->suffix);
 		proto = arc_bcast_proto;
 	}
-	return proto->build_header(skb, type, _daddr);
+	return proto->build_header(skb, dev, type, _daddr);
 }
 
 
@@ -495,6 +495,7 @@ static int arcnet_rebuild_header(struct 
 	int status = 0;		/* default is failure */
 	unsigned short type;
 	uint8_t daddr=0;
+	struct ArcProto *proto;
 
 	if (skb->nh.raw - skb->mac.raw != 2) {
 		BUGMSG(D_NORMAL,
@@ -523,7 +524,8 @@ static int arcnet_rebuild_header(struct 
 		return 0;
 
 	/* add the _real_ header this time! */
-	arc_proto_map[lp->default_proto[daddr]]->build_header(skb, type, daddr);
+	proto = arc_proto_map[lp->default_proto[daddr]];
+	proto->build_header(skb, dev, type, daddr);
 
 	return 1;		/* success */
 }
@@ -952,10 +954,9 @@ static void null_rx(struct net_device *d
 }
 
 
-static int null_build_header(struct sk_buff *skb, unsigned short type,
-			     uint8_t daddr)
+static int null_build_header(struct sk_buff *skb, struct net_device *dev,
+			     unsigned short type, uint8_t daddr)
 {
-	struct net_device *dev = skb->dev;
 	struct arcnet_local *lp = (struct arcnet_local *) dev->priv;
 
 	BUGMSG(D_PROTO,
diff -puN drivers/net/arcnet/arc-rawmode.c~arcnet-oops-fix drivers/net/arcnet/arc-rawmode.c
--- linux-2.5.70-ds/drivers/net/arcnet/arc-rawmode.c~arcnet-oops-fix	2003-06-08 02:09:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/net/arcnet/arc-rawmode.c	2003-06-08 02:09:38.000000000 +0530
@@ -37,8 +37,8 @@
 
 static void rx(struct net_device *dev, int bufnum,
 	       struct archdr *pkthdr, int length);
-static int build_header(struct sk_buff *skb, unsigned short type,
-			uint8_t daddr);
+static int build_header(struct sk_buff *skb, struct net_device *dev,
+			unsigned short type, uint8_t daddr);
 static int prepare_tx(struct net_device *dev, struct archdr *pkt, int length,
 		      int bufnum);
 
@@ -131,10 +131,9 @@ static void rx(struct net_device *dev, i
  * Create the ARCnet hard/soft headers for raw mode.
  * There aren't any soft headers in raw mode - not even the protocol id.
  */
-static int build_header(struct sk_buff *skb, unsigned short type,
-			uint8_t daddr)
+static int build_header(struct sk_buff *skb, struct net_device *dev,
+			unsigned short type, uint8_t daddr)
 {
-	struct net_device *dev = skb->dev;
 	int hdr_size = ARC_HDR_SIZE;
 	struct archdr *pkt = (struct archdr *) skb_push(skb, hdr_size);
 
diff -puN drivers/net/arcnet/rfc1051.c~arcnet-oops-fix drivers/net/arcnet/rfc1051.c
--- linux-2.5.70-ds/drivers/net/arcnet/rfc1051.c~arcnet-oops-fix	2003-06-08 02:09:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/net/arcnet/rfc1051.c	2003-06-08 02:09:38.000000000 +0530
@@ -37,8 +37,8 @@
 static unsigned short type_trans(struct sk_buff *skb, struct net_device *dev);
 static void rx(struct net_device *dev, int bufnum,
 	       struct archdr *pkthdr, int length);
-static int build_header(struct sk_buff *skb, unsigned short type,
-			uint8_t daddr);
+static int build_header(struct sk_buff *skb, struct net_device *dev,
+			unsigned short type, uint8_t daddr);
 static int prepare_tx(struct net_device *dev, struct archdr *pkt, int length,
 		      int bufnum);
 
@@ -163,10 +163,9 @@ static void rx(struct net_device *dev, i
 /*
  * Create the ARCnet hard/soft headers for RFC1051.
  */
-static int build_header(struct sk_buff *skb, unsigned short type,
-			uint8_t daddr)
+static int build_header(struct sk_buff *skb, struct net_device *dev,
+			unsigned short type, uint8_t daddr)
 {
-	struct net_device *dev = skb->dev;
 	struct arcnet_local *lp = (struct arcnet_local *) dev->priv;
 	int hdr_size = ARC_HDR_SIZE + RFC1051_HDR_SIZE;
 	struct archdr *pkt = (struct archdr *) skb_push(skb, hdr_size);
diff -puN drivers/net/arcnet/rfc1201.c~arcnet-oops-fix drivers/net/arcnet/rfc1201.c
--- linux-2.5.70-ds/drivers/net/arcnet/rfc1201.c~arcnet-oops-fix	2003-06-08 02:09:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/net/arcnet/rfc1201.c	2003-06-08 02:09:38.000000000 +0530
@@ -36,8 +36,8 @@
 static unsigned short type_trans(struct sk_buff *skb, struct net_device *dev);
 static void rx(struct net_device *dev, int bufnum,
 	       struct archdr *pkthdr, int length);
-static int build_header(struct sk_buff *skb, unsigned short type,
-			uint8_t daddr);
+static int build_header(struct sk_buff *skb, struct net_device *dev,
+			unsigned short type, uint8_t daddr);
 static int prepare_tx(struct net_device *dev, struct archdr *pkt, int length,
 		      int bufnum);
 static int continue_tx(struct net_device *dev, int bufnum);
@@ -370,10 +370,9 @@ static void rx(struct net_device *dev, i
 
 
 /* Create the ARCnet hard/soft headers for RFC1201. */
-static int build_header(struct sk_buff *skb, unsigned short type,
-			uint8_t daddr)
+static int build_header(struct sk_buff *skb, struct net_device *dev,
+			unsigned short type, uint8_t daddr)
 {
-	struct net_device *dev = skb->dev;
 	struct arcnet_local *lp = (struct arcnet_local *) dev->priv;
 	int hdr_size = ARC_HDR_SIZE + RFC1201_HDR_SIZE;
 	struct archdr *pkt = (struct archdr *) skb_push(skb, hdr_size);
diff -puN include/linux/arcdevice.h~arcnet-oops-fix include/linux/arcdevice.h
--- linux-2.5.70-ds/include/linux/arcdevice.h~arcnet-oops-fix	2003-06-08 02:09:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/include/linux/arcdevice.h	2003-06-08 02:09:38.000000000 +0530
@@ -190,8 +190,8 @@ struct ArcProto {
 
 	void (*rx) (struct net_device * dev, int bufnum,
 		    struct archdr * pkthdr, int length);
-	int (*build_header) (struct sk_buff * skb, unsigned short ethproto,
-			     uint8_t daddr);
+	int (*build_header) (struct sk_buff * skb, struct net_device *dev,
+			     unsigned short ethproto, uint8_t daddr);
 
 	/* these functions return '1' if the skb can now be freed */
 	int (*prepare_tx) (struct net_device * dev, struct archdr * pkt, int length,

_

Re: Misc 2.5 Fixes: x25-facilities-parse

[Dipankar Sarma]

Fix parsing of options for X.25 facilities


 net/x25/x25_facilities.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN net/x25/x25_facilities.c~x25-facilities-parse net/x25/x25_facilities.c
--- linux-2.5.70-ds/net/x25/x25_facilities.c~x25-facilities-parse	2003-06-08 00:39:39.000000000 +0530
+++ linux-2.5.70-ds-dipankar/net/x25/x25_facilities.c	2003-06-08 00:40:28.000000000 +0530
@@ -105,8 +105,8 @@ int x25_parse_facilities(struct sk_buff 
 			printk(KERN_DEBUG "X.25: unknown facility %02X, "
 			       "length %d, values %02X, %02X, %02X, %02X\n",
 			       p[0], p[1], p[2], p[3], p[4], p[5]);
-			p   += p[1] + 2;
 			len -= p[1] + 2;
+			p   += p[1] + 2;
 			break;
 		}
 	}

_

Re: Misc 2.5 Fixes: cp-user-awe

[Dipankar Sarma]

Fix copy/user in awe_wave.


 sound/oss/awe_wave.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -puN sound/oss/awe_wave.c~cp-user-awe sound/oss/awe_wave.c
--- linux-2.5.70-ds/sound/oss/awe_wave.c~cp-user-awe	2003-06-08 20:49:13.000000000 +0530
+++ linux-2.5.70-ds-dipankar/sound/oss/awe_wave.c	2003-06-08 20:49:13.000000000 +0530
@@ -2046,9 +2046,9 @@ awe_ioctl(int dev, unsigned int cmd, cad
 			awe_info.nr_voices = awe_max_voices;
 		else
 			awe_info.nr_voices = AWE_MAX_CHANNELS;
-		memcpy((char*)arg, &awe_info, sizeof(awe_info));
+		if(copy_to_user(arg, &awe_info, sizeof(awe_info)))
+			return -EFAULT;
 		return 0;
-		break;
 
 	case SNDCTL_SEQ_RESETSAMPLES:
 		awe_reset(dev);

_

Re: Misc 2.5 Fixes: cp-user-cmpci

[Dipankar Sarma]

Fix copy/user problems. Not sure why cm_write() needs to do
acces_ok() on buffer twice. Also __get_user() return value isn't checked
in trans_ac3().


 sound/oss/cmpci.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff -puN sound/oss/cmpci.c~cp-user-cmpci sound/oss/cmpci.c
--- linux-2.5.70-ds/sound/oss/cmpci.c~cp-user-cmpci	2003-06-08 15:36:16.000000000 +0530
+++ linux-2.5.70-ds-dipankar/sound/oss/cmpci.c	2003-06-08 20:39:03.000000000 +0530
@@ -588,7 +588,8 @@ static void trans_ac3(struct cm_state *s
 	unsigned short *src = (unsigned short *)source;
 
 	do {
-		data = (unsigned long) *src++;
+		__get_user(data, src);
+		src++;
 		data <<= 12;			// ok for 16-bit data
 		if (s->spdif_counter == 2 || s->spdif_counter == 3)
 			data |= 0x40000000;	// indicate AC-3 raw data
@@ -1600,9 +1601,9 @@ static ssize_t cm_write(struct file *fil
 			return -ENXIO;
 		if (!s->dma_adc.ready && (ret = prog_dmabuf(s, 1)))
 			return ret;
-		if (!access_ok(VERIFY_READ, buffer, count))
-			return -EFAULT;
 	}
+	if (!access_ok(VERIFY_READ, buffer, count))
+		return -EFAULT;
 	ret = 0;
 
 	while (count > 0) {
@@ -1662,15 +1663,21 @@ static ssize_t cm_write(struct file *fil
 			swptr = (swptr + 2 * cnt) % s->dma_dac.dmasize;
 		} else if (s->status & DO_DUAL_DAC) {
 			int	i;
-			unsigned long *src, *dst0, *dst1;
+			unsigned long *src, *dst0, *dst1, data;
 
 			src = (unsigned long *) buffer;
 			dst0 = (unsigned long *) (s->dma_dac.rawbuf + swptr);
 			dst1 = (unsigned long *) (s->dma_adc.rawbuf + swptr);
 			// copy left/right sample at one time
 			for (i = 0; i <= cnt / 4; i++) {
-				*dst0++ = *src++;
-				*dst1++ = *src++;
+				if (__get_user(data, src))
+					return ret ? ret : -EFAULT;
+				*dst0++ = data;
+				src++;
+				if (__get_user(data, src))
+					return ret ? ret : -EFAULT;
+				*dst1++ = data;
+				src++;
 			}
 			swptr = (swptr + cnt) % s->dma_dac.dmasize;
 		} else {

_

Re: Misc 2.5 Fixes: cp-user-eicon

[Dipankar Sarma]

Use copy_to_user, not memcpy with user buffers


 drivers/isdn/eicon/linchr.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff -puN drivers/isdn/eicon/linchr.c~cp-user-eicon drivers/isdn/eicon/linchr.c
--- linux-2.5.70-ds/drivers/isdn/eicon/linchr.c~cp-user-eicon	2003-06-08 03:30:31.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/isdn/eicon/linchr.c	2003-06-08 03:30:31.000000000 +0530
@@ -153,17 +153,17 @@ ssize_t do_read(struct file *pFile, char
 	klog_t *pHeadItem;
 
 	if (BufferSize < sizeof(klog_t))
-	{
-		printk(KERN_WARNING "Divas: Divalog buffer specifed a size that is too small (%d - %d required)\n",
-			BufferSize, sizeof(klog_t));
 		return -EIO;
-	}
 
 	pHeadItem = (klog_t *) DivasLogFifoRead();
 
 	if (pHeadItem)
 	{
-		memcpy(pClientLogBuffer, pHeadItem, sizeof(klog_t));
+		if(copy_to_user(pClientLogBuffer, pHeadItem, sizeof(klog_t)))
+		{
+			kfree(pHeadItem);
+			return -EFAULT;
+		}
 		kfree(pHeadItem);
 		return sizeof(klog_t);
 	}

_

Re: Misc 2.5 Fixes: Summary

[Joshua Kwan]

[-- Attachment #1: Type: text/plain, Size: 423 bytes --]

On Tue, Jun 10, 2003 at 03:05:44AM -0700, Joshua Kwan wrote:
> On Tue, Jun 10, 2003 at 03:35:27PM +0530, Dipankar Sarma wrote:
> > This set of patches fix various problems mostly related to
> > copy/user problems and leaks. I worked based on Alan's list and
> 
> What set of patches?

Sorry, the second I sent that off, I received the first few messages..
My bad :)

-Josh

-- 
New PGP public key: 0x27AFC3EE

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Misc 2.5 Fixes: cp-user-intermezzo

[Dipankar Sarma]

Fixed copy/user problem in lento_symlink where user address was
getting passed to presto_do_symlink.


 fs/intermezzo/vfs.c |    2 +-
 1 files changed, 1 insertion(+), 1 deletion(-)

diff -puN fs/intermezzo/vfs.c~cp-user-intermezzo fs/intermezzo/vfs.c
--- linux-2.5.70-ds/fs/intermezzo/vfs.c~cp-user-intermezzo	2003-06-08 14:07:49.000000000 +0530
+++ linux-2.5.70-ds-dipankar/fs/intermezzo/vfs.c	2003-06-08 14:09:28.000000000 +0530
@@ -1236,7 +1236,7 @@ int lento_symlink(const char *oldname, c
                 goto exit_lock;
         }
         error = presto_do_symlink(fset, nd.dentry,
-                                  dentry, oldname, info);
+                                  dentry, from, info);
         path_release(&nd);
         EXIT;
  exit_lock:

_

Re: Misc 2.5 Fixes: cp-user-mdc800

[Dipankar Sarma]

Use copy_to_user/get_char with user buffers.


 drivers/usb/image/mdc800.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff -puN drivers/usb/image/mdc800.c~cp-user-mdc800 drivers/usb/image/mdc800.c
--- linux-2.5.70-ds/drivers/usb/image/mdc800.c~cp-user-mdc800	2003-06-08 03:13:41.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/usb/image/mdc800.c	2003-06-08 03:21:18.000000000 +0530
@@ -748,8 +748,10 @@ static ssize_t mdc800_device_read (struc
 		}
 		else
 		{
-			/* memcpy Bytes */
-			memcpy (ptr, &mdc800->out [mdc800->out_ptr], sts);
+			/* Copy Bytes */
+			if (copy_to_user(ptr, 
+				&mdc800->out [mdc800->out_ptr], sts))
+				return -EFAULT;
 			ptr+=sts;
 			left-=sts;
 			mdc800->out_ptr+=sts;
@@ -786,14 +788,21 @@ static ssize_t mdc800_device_write (stru
 
 	while (i<len)
 	{
+		unsigned char c;
 		if (signal_pending (current)) 
 		{
 			up (&mdc800->io_lock);
 			return -EINTR;
 		}
+		
+		if(get_user(c, buf+i))
+		{
+			up(&mdc800->io_lock);
+			return -EFAULT;
+		}
 
 		/* check for command start */
-		if (buf [i] == (char) 0x55)
+		if (c == 0x55)
 		{
 			mdc800->in_count=0;
 			mdc800->out_count=0;
@@ -804,12 +813,11 @@ static ssize_t mdc800_device_write (stru
 		/* save command byte */
 		if (mdc800->in_count < 8)
 		{
-			mdc800->in[mdc800->in_count]=buf[i];
+			mdc800->in[mdc800->in_count] = c;
 			mdc800->in_count++;
 		}
 		else
 		{
-			err ("Command is too long !\n");
 			up (&mdc800->io_lock);
 			return -EIO;
 		}
@@ -884,8 +892,8 @@ static ssize_t mdc800_device_write (stru
 							return -EIO;
 						}
 
-						/* Write dummy data, (this is ugly but part of the USB Protokoll */
-						/* if you use endpoint 1 as bulk and not as irq */
+						/* Write dummy data, (this is ugly but part of the USB Protocol */
+						/* if you use endpoint 1 as bulk and not as irq) */
 						memcpy (mdc800->out, mdc800->camera_response,8);
 
 						/* This is the interpreted answer */

_

Re: Misc 2.5 Fixes: cp-user-mpu401

[Dipankar Sarma]

Use copy_to_user to copy mpu_synth_ioctl arg.


 sound/oss/mpu401.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

diff -puN sound/oss/mpu401.c~cp-user-mpu401 sound/oss/mpu401.c
--- linux-2.5.70-ds/sound/oss/mpu401.c~cp-user-mpu401	2003-06-08 02:59:48.000000000 +0530
+++ linux-2.5.70-ds-dipankar/sound/oss/mpu401.c	2003-06-08 02:59:48.000000000 +0530
@@ -792,7 +792,8 @@ static int mpu_synth_ioctl(int dev,
 	{
 
 		case SNDCTL_SYNTH_INFO:
-			memcpy((&((char *) arg)[0]), (char *) &mpu_synth_info[midi_dev], sizeof(struct synth_info));
+			if(copy_to_user((&((char *) arg)[0]), (char *) &mpu_synth_info[midi_dev], sizeof(struct synth_info)))
+				return -EFAULT;
 			return 0;
 
 		case SNDCTL_SYNTH_MEMAVL:

_

Re: Misc 2.5 Fixes: cp-user-sisfb

[Dipankar Sarma]

Fix sisfb_ioctl() to use copy_to/from routines. There may be some
some changes in this patch that are ifdefed out in 2.5. Maintainers
to rescue.


 drivers/video/sis/sis_main.c |   91 +++++++++++++++++++++++++------------------
 1 files changed, 55 insertions(+), 36 deletions(-)

diff -puN drivers/video/sis/sis_main.c~cp-user-sisfb drivers/video/sis/sis_main.c
--- linux-2.5.70-ds/drivers/video/sis/sis_main.c~cp-user-sisfb	2003-06-08 04:34:39.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/video/sis/sis_main.c	2003-06-08 12:27:49.000000000 +0530
@@ -1461,44 +1461,57 @@ static int sisfb_ioctl(struct inode *ino
 		       struct fb_info *info)
 {
 	TWDEBUG("inside ioctl");
+	struct sis_memreq req;
+	struct ap_data ap;
+	unsigned long a;
 	switch (cmd) {
 	   case FBIO_ALLOC:
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
-		sis_malloc((struct sis_memreq *) arg);
+		if (copy_from_user(&req, (void *)arg, sizeof(req)))
+			return -EFAULT;
+		sis_malloc(&req);
+		if (copy_to_user((void *)arg, &req, sizeof(req)))
+			return -EFAULT;
 		break;
 	   case FBIO_FREE:
 		if (!capable(CAP_SYS_RAWIO))
 			return -EPERM;
-		sis_free(*(unsigned long *) arg);
+		if(get_user(a, (unsigned long *) arg))
+			return -EFAULT;
+		sis_free(a);
 		break;
 	   case FBIOGET_GLYPH:
+		/* Not in 2.5 ???? */
                 sis_get_glyph(info,(SIS_GLYINFO *) arg);
 		break;	
 	   case FBIOGET_HWCINFO:
 		{
 			unsigned long *hwc_offset = (unsigned long *) arg;
 
-			if (sisfb_caps & HW_CURSOR_CAP)
-				*hwc_offset = sisfb_hwcursor_vbase -
-				    (unsigned long) ivideo.video_vbase;
-			else
-				*hwc_offset = 0;
-
+			if (sisfb_caps & HW_CURSOR_CAP) {
+				if (put_user(sisfb_hwcursor_vbase -
+				    (unsigned long) ivideo.video_vbase,
+					hwc_offset))
+					return -EFAULT;
+			} else if (put_user(0UL, hwc_offset))
+					return -EFAULT;
 			break;
 		}
 	   case FBIOPUT_MODEINFO:
 		{
-			struct mode_info *x = (struct mode_info *)arg;
+			struct mode_info x;
 
-			ivideo.video_bpp        = x->bpp;
-			ivideo.video_width      = x->xres;
-			ivideo.video_height     = x->yres;
-			ivideo.video_vwidth     = x->v_xres;
-			ivideo.video_vheight    = x->v_yres;
-			ivideo.org_x            = x->org_x;
-			ivideo.org_y            = x->org_y;
-			ivideo.refresh_rate     = x->vrate;
+			if (copy_from_user(&x, (void *)arg, sizeof(x)))
+				return -EFAULT;
+			ivideo.video_bpp        = x.bpp;
+			ivideo.video_width      = x.xres;
+			ivideo.video_height     = x.yres;
+			ivideo.video_vwidth     = x.v_xres;
+			ivideo.video_vheight    = x.v_yres;
+			ivideo.org_x            = x.org_x;
+			ivideo.org_y            = x.org_y;
+			ivideo.refresh_rate     = x.vrate;
 			ivideo.video_linelength = ivideo.video_vwidth * (ivideo.video_bpp >> 3);
 			switch(ivideo.video_bpp) {
         		case 8:
@@ -1526,34 +1539,40 @@ static int sisfb_ioctl(struct inode *ino
 			break;
 		}
 	   case FBIOGET_DISPINFO:
-		sis_dispinfo((struct ap_data *)arg);
+		sis_dispinfo(&ap);
+		if (copy_to_user((void *)arg, &ap, sizeof(ap)))
+			return -EFAULT;
 		break;
 	   case SISFB_GET_INFO:  /* TW: New for communication with X driver */
 	        {
-			sisfb_info *x = (sisfb_info *)arg;
+			sisfb_info x;
 
-			x->sisfb_id = SISFB_ID;
-			x->sisfb_version = VER_MAJOR;
-			x->sisfb_revision = VER_MINOR;
-			x->sisfb_patchlevel = VER_LEVEL;
-			x->chip_id = ivideo.chip_id;
-			x->memory = ivideo.video_size / 1024;
-			x->heapstart = ivideo.heapstart / 1024;
-			x->fbvidmode = sisfb_mode_no;
-			x->sisfb_caps = sisfb_caps;
-			x->sisfb_tqlen = 512; /* yet unused */
-			x->sisfb_pcibus = ivideo.pcibus;
-			x->sisfb_pcislot = ivideo.pcislot;
-			x->sisfb_pcifunc = ivideo.pcifunc;
-			x->sisfb_lcdpdc = sisfb_detectedpdc;
-			x->sisfb_lcda = sisfb_detectedlcda;
+			x.sisfb_id = SISFB_ID;
+			x.sisfb_version = VER_MAJOR;
+			x.sisfb_revision = VER_MINOR;
+			x.sisfb_patchlevel = VER_LEVEL;
+			x.chip_id = ivideo.chip_id;
+			x.memory = ivideo.video_size / 1024;
+			x.heapstart = ivideo.heapstart / 1024;
+			x.fbvidmode = sisfb_mode_no;
+			x.sisfb_caps = sisfb_caps;
+			x.sisfb_tqlen = 512; /* yet unused */
+			x.sisfb_pcibus = ivideo.pcibus;
+			x.sisfb_pcislot = ivideo.pcislot;
+			x.sisfb_pcifunc = ivideo.pcifunc;
+			x.sisfb_lcdpdc = sisfb_detectedpdc;
+			x.sisfb_lcda = sisfb_detectedlcda;
+			if (copy_to_user((void *)arg, &x, sizeof(x)))
+				return -EFAULT;
 	                break;
 		}
 	   case SISFB_GET_VBRSTATUS:
 	        {
 			unsigned long *vbrstatus = (unsigned long *) arg;
-			if(sisfb_CheckVBRetrace()) *vbrstatus = 1;
-			else		           *vbrstatus = 0;
+			if(sisfb_CheckVBRetrace())  {
+				return put_user(1UL, vbrstatus);
+			else		           
+				return put_user(0UL, vbrstatus);
 		}
 	   default:
 		return -EINVAL;

_

Re: Misc 2.5 Fixes: cp-user-vicam

[Dipankar Sarma]

Fix handling of user bufs (arg), use copy_from_user.


 drivers/usb/media/vicam.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff -puN drivers/usb/media/vicam.c~cp-user-vicam drivers/usb/media/vicam.c
--- linux-2.5.70-ds/drivers/usb/media/vicam.c~cp-user-vicam	2003-06-08 03:55:24.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/usb/media/vicam.c	2003-06-08 04:03:39.000000000 +0530
@@ -611,7 +611,12 @@ vicam_ioctl(struct inode *inode, struct 
 
 	case VIDIOCSPICT:
 		{
-			struct video_picture *vp = (struct video_picture *) arg;
+			struct video_picture vp;
+
+			if (copy_from_user(&vp, arg, sizeof (vp))) {
+				retval = -EFAULT;
+				break;
+			}
 
 			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp->depth,
 			    vp->palette);
@@ -652,7 +657,12 @@ vicam_ioctl(struct inode *inode, struct 
 	case VIDIOCSWIN:
 		{
 
-			struct video_window *vw = (struct video_window *) arg;
+			struct video_window vw;
+
+			if (copy_from_user(&vw, arg, sizeof (vw))) {
+				retval = -EFAULT;
+				break;
+			}
 			DBG("VIDIOCSWIN %d x %d\n", vw->width, vw->height);
 
 			if ( vw->width != 320 || vw->height != 240 )

_

Re: Misc 2.5 Fixes: cp-user-zoran

[Dipankar Sarma]

Use copy_to_user/put_user with user buffers.


 drivers/media/video/zoran_procfs.c |    6 +++++-
 drivers/media/video/zr36120.c      |   12 ++++++------
 2 files changed, 11 insertions(+), 7 deletions(-)

diff -puN drivers/media/video/zoran_procfs.c~cp-user-zoran drivers/media/video/zoran_procfs.c
--- linux-2.5.70-ds/drivers/media/video/zoran_procfs.c~cp-user-zoran	2003-06-08 04:10:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/media/video/zoran_procfs.c	2003-06-08 04:10:38.000000000 +0530
@@ -119,7 +119,11 @@ static int zoran_write_proc(struct file 
 		printk(KERN_ERR "%s: write_proc: can not allocate memory\n", zr->name);
 		return -ENOMEM;
 	}
-	memcpy(string, buffer, count);
+	if(copy_from_user(string, buffer, count))
+	{
+		vfree(string);
+		return -EFAULT;
+	}
 	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";
diff -puN drivers/media/video/zr36120.c~cp-user-zoran drivers/media/video/zr36120.c
--- linux-2.5.70-ds/drivers/media/video/zr36120.c~cp-user-zoran	2003-06-08 04:10:38.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/media/video/zr36120.c	2003-06-08 04:10:38.000000000 +0530
@@ -1693,12 +1693,12 @@ long vbi_read(struct video_device* dev, 
 			for (x=0; optr+1<eptr && x<-done->w; x++)
 			{
 				unsigned char a = iptr[x*2];
-				*optr++ = a;
-				*optr++ = a;
+				__put_user(a, optr++);
+				__put_user(a, optr++);
 			}
 			/* and clear the rest of the line */
 			for (x*=2; optr<eptr && x<done->bpl; x++)
-				*optr++ = 0;
+				__put_user(0, optr++);
 			/* next line */
 			iptr += done->bpl;
 		}
@@ -1715,10 +1715,10 @@ long vbi_read(struct video_device* dev, 
 		{
 			/* copy to doubled data to userland */
 			for (x=0; optr<eptr && x<-done->w; x++)
-				*optr++ = iptr[x*2];
+				__put_user(iptr[x*2], optr++);
 			/* and clear the rest of the line */
 			for (;optr<eptr && x<done->bpl; x++)
-				*optr++ = 0;
+				__put_user(0, optr++);
 			/* next line */
 			iptr += done->bpl;
 		}
@@ -1727,7 +1727,7 @@ long vbi_read(struct video_device* dev, 
 	/* API compliance:
 	 * place the framenumber (half fieldnr) in the last long
 	 */
-	((ulong*)eptr)[-1] = done->fieldnr/2;
+	__put_user(done->fieldnr/2, ((ulong*)eptr)-1);
 	}
 
 	/* keep the engine running */

_

Re: Misc 2.5 Fixes: mem-leak-emu10k1

[Dipankar Sarma]

Fix memory leak in emu10k1_audio_open.


 sound/oss/emu10k1/audio.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletion(-)

diff -puN sound/oss/emu10k1/audio.c~mem-leak-emu10k1 sound/oss/emu10k1/audio.c
--- linux-2.5.70-ds/sound/oss/emu10k1/audio.c~mem-leak-emu10k1	2003-06-08 22:55:56.000000000 +0530
+++ linux-2.5.70-ds-dipankar/sound/oss/emu10k1/audio.c	2003-06-08 22:57:33.000000000 +0530
@@ -1187,7 +1187,8 @@ match:
 
 		if ((woinst = (struct woinst *) kmalloc(sizeof(struct woinst), GFP_KERNEL)) == NULL) {
 			ERROR();
-			return -ENODEV;
+			kfree(wave_dev);
+			return -ENOMEM;
 		}
 
 		if (wave_dev->wiinst != NULL) {

_

Re: Misc 2.5 Fixes: mem-leak-rio

[Dipankar Sarma]

Fix memory leak - free on copyin failure.


 drivers/char/rio/rioboot.c |    1 +
 1 files changed, 1 insertion(+)

diff -puN drivers/char/rio/rioboot.c~mem-leak-rio drivers/char/rio/rioboot.c
--- linux-2.5.70-ds/drivers/char/rio/rioboot.c~mem-leak-rio	2003-06-08 21:21:50.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/char/rio/rioboot.c	2003-06-08 21:21:50.000000000 +0530
@@ -326,6 +326,7 @@ register struct DownLoad *rbp;
 
 			if ( copyin((int)rbp->DataP,DownCode,rbp->Count)==COPYFAIL ) {
 				rio_dprintk (RIO_DEBUG_BOOT, "Bad copyin of host data\n");
+				sysfree( DownCode, rbp->Count );
 				p->RIOError.Error = COPYIN_FAILED;
 				func_exit ();
 				return -EFAULT;

_

Re: Misc 2.5 Fixes: resrc-leak-i810

[Dipankar Sarma]

Free any read channel allocated earlier if allocation of write channel
fails.


 sound/oss/i810_audio.c |    5 +++++
 1 files changed, 5 insertions(+)

diff -puN sound/oss/i810_audio.c~resrc-leak-i810 sound/oss/i810_audio.c
--- linux-2.5.70-ds/sound/oss/i810_audio.c~resrc-leak-i810	2003-06-09 22:10:21.000000000 +0530
+++ linux-2.5.70-ds-dipankar/sound/oss/i810_audio.c	2003-06-09 22:16:04.000000000 +0530
@@ -2493,6 +2493,11 @@ found_virt:
 	}
 	if(file->f_mode & FMODE_WRITE) {
 		if((dmabuf->write_channel = card->alloc_pcm_channel(card)) == NULL) {
+			/* free any read channel allocated earlier */
+			if(file->f_mode & FMODE_READ)
+				card->free_pcm_channel(card,
+						dmabuf->read_channel->num);
+
 			kfree (card->states[i]);
 			card->states[i] = NULL;;
 			return -EBUSY;

_

Re: Misc 2.5 Fixes: cp-user-cmpci

[Hollis Blanchard]

On Tuesday, Jun 10, 2003, at 05:09 US/Central, Dipankar Sarma wrote:
>
> Fix copy/user problems. Not sure why cm_write() needs to do
> acces_ok() on buffer twice. Also __get_user() return value isn't 
> checked
> in trans_ac3().

Hey Dipankar, this file has been fixed in bitkeeper.

>  sound/oss/cmpci.c |   19 +++++++++++++------
>  1 files changed, 13 insertions(+), 6 deletions(-)
>
> diff -puN sound/oss/cmpci.c~cp-user-cmpci sound/oss/cmpci.c
> --- linux-2.5.70-ds/sound/oss/cmpci.c~cp-user-cmpci	2003-06-08 
> 15:36:16.000000000 +0530
> +++ linux-2.5.70-ds-dipankar/sound/oss/cmpci.c	2003-06-08 
> 20:39:03.000000000 +0530
> @@ -588,7 +588,8 @@ static void trans_ac3(struct cm_state *s
>  	unsigned short *src = (unsigned short *)source;
>
>  	do {
> -		data = (unsigned long) *src++;
> +		__get_user(data, src);
> +		src++;
>  		data <<= 12;			// ok for 16-bit data
>  		if (s->spdif_counter == 2 || s->spdif_counter == 3)
>  			data |= 0x40000000;	// indicate AC-3 raw data

Above you mentioned that __get_user isn't checked, but it clearly 
should be. trans_ac3 has been made to return an error code.

> @@ -1600,9 +1601,9 @@ static ssize_t cm_write(struct file *fil
>  			return -ENXIO;
>  		if (!s->dma_adc.ready && (ret = prog_dmabuf(s, 1)))
>  			return ret;
> -		if (!access_ok(VERIFY_READ, buffer, count))
> -			return -EFAULT;
>  	}
> +	if (!access_ok(VERIFY_READ, buffer, count))
> +		return -EFAULT;
>  	ret = 0;
>
>  	while (count > 0) {

Good catch. However if I'm reading it right you still have two 
access_ok calls (the other is a few lines above this patch context).

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: Misc 2.5 Fixes: cp-user-zoran

[Hollis Blanchard]

On Tuesday, Jun 10, 2003, at 05:22 US/Central, Dipankar Sarma wrote:
>
> diff -puN drivers/media/video/zr36120.c~cp-user-zoran 
> drivers/media/video/zr36120.c
> --- 
> linux-2.5.70-ds/drivers/media/video/zr36120.c~cp-user-zoran	2003-06-08 
> 04:10:38.000000000 +0530
> +++ linux-2.5.70-ds-dipankar/drivers/media/video/zr36120.c	2003-06-08 
> 04:10:38.000000000 +0530
> @@ -1693,12 +1693,12 @@ long vbi_read(struct video_device* dev,
>  			for (x=0; optr+1<eptr && x<-done->w; x++)
>  			{
>  				unsigned char a = iptr[x*2];
> -				*optr++ = a;
> -				*optr++ = a;
> +				__put_user(a, optr++);
> +				__put_user(a, optr++);
>  			}
>  			/* and clear the rest of the line */
>  			for (x*=2; optr<eptr && x<done->bpl; x++)
> -				*optr++ = 0;
> +				__put_user(0, optr++);
>  			/* next line */
>  			iptr += done->bpl;
>  		}
> @@ -1715,10 +1715,10 @@ long vbi_read(struct video_device* dev,
>  		{
>  			/* copy to doubled data to userland */
>  			for (x=0; optr<eptr && x<-done->w; x++)
> -				*optr++ = iptr[x*2];
> +				__put_user(iptr[x*2], optr++);
>  			/* and clear the rest of the line */
>  			for (;optr<eptr && x<done->bpl; x++)
> -				*optr++ = 0;
> +				__put_user(0, optr++);
>  			/* next line */
>  			iptr += done->bpl;
>  		}
> @@ -1727,7 +1727,7 @@ long vbi_read(struct video_device* dev,
>  	/* API compliance:
>  	 * place the framenumber (half fieldnr) in the last long
>  	 */
> -	((ulong*)eptr)[-1] = done->fieldnr/2;
> +	__put_user(done->fieldnr/2, ((ulong*)eptr)-1);
>  	}
>
>  	/* keep the engine running */

It's funny, I did the exact same thing for the version currently in 
bk... but I just realized that __put_user still returns an error code, 
so all those calls should be checked.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: Misc 2.5 Fixes: cp-user-sisfb

[Thomas Winischhofer]

Thanks, but this was already fixed in my latest version. I really should 
commit more often...

Thomas

Dipankar Sarma wrote:
> 
> Fix sisfb_ioctl() to use copy_to/from routines. There may be some
> some changes in this patch that are ifdefed out in 2.5. Maintainers
> to rescue.
> 
> 
>  drivers/video/sis/sis_main.c |   91 +++++++++++++++++++++++++------------------
>  1 files changed, 55 insertions(+), 36 deletions(-)
> 
> diff -puN drivers/video/sis/sis_main.c~cp-user-sisfb drivers/video/sis/sis_main.c
> --- linux-2.5.70-ds/drivers/video/sis/sis_main.c~cp-user-sisfb	2003-06-08 04:34:39.000000000 +0530
> +++ linux-2.5.70-ds-dipankar/drivers/video/sis/sis_main.c	2003-06-08 12:27:49.000000000 +0530
> @@ -1461,44 +1461,57 @@ static int sisfb_ioctl(struct inode *ino
>  		       struct fb_info *info)
>  {
>  	TWDEBUG("inside ioctl");
> +	struct sis_memreq req;
> +	struct ap_data ap;
> +	unsigned long a;
>  	switch (cmd) {
>  	   case FBIO_ALLOC:
>  		if (!capable(CAP_SYS_RAWIO))
>  			return -EPERM;
> -		sis_malloc((struct sis_memreq *) arg);
> +		if (copy_from_user(&req, (void *)arg, sizeof(req)))
> +			return -EFAULT;
> +		sis_malloc(&req);
> +		if (copy_to_user((void *)arg, &req, sizeof(req)))
> +			return -EFAULT;
>  		break;
>  	   case FBIO_FREE:
>  		if (!capable(CAP_SYS_RAWIO))
>  			return -EPERM;
> -		sis_free(*(unsigned long *) arg);
> +		if(get_user(a, (unsigned long *) arg))
> +			return -EFAULT;
> +		sis_free(a);
>  		break;
>  	   case FBIOGET_GLYPH:
> +		/* Not in 2.5 ???? */
>                  sis_get_glyph(info,(SIS_GLYINFO *) arg);
>  		break;	
>  	   case FBIOGET_HWCINFO:
>  		{
>  			unsigned long *hwc_offset = (unsigned long *) arg;
>  
> -			if (sisfb_caps & HW_CURSOR_CAP)
> -				*hwc_offset = sisfb_hwcursor_vbase -
> -				    (unsigned long) ivideo.video_vbase;
> -			else
> -				*hwc_offset = 0;
> -
> +			if (sisfb_caps & HW_CURSOR_CAP) {
> +				if (put_user(sisfb_hwcursor_vbase -
> +				    (unsigned long) ivideo.video_vbase,
> +					hwc_offset))
> +					return -EFAULT;
> +			} else if (put_user(0UL, hwc_offset))
> +					return -EFAULT;
>  			break;
>  		}
>  	   case FBIOPUT_MODEINFO:
>  		{
> -			struct mode_info *x = (struct mode_info *)arg;
> +			struct mode_info x;
>  
> -			ivideo.video_bpp        = x->bpp;
> -			ivideo.video_width      = x->xres;
> -			ivideo.video_height     = x->yres;
> -			ivideo.video_vwidth     = x->v_xres;
> -			ivideo.video_vheight    = x->v_yres;
> -			ivideo.org_x            = x->org_x;
> -			ivideo.org_y            = x->org_y;
> -			ivideo.refresh_rate     = x->vrate;
> +			if (copy_from_user(&x, (void *)arg, sizeof(x)))
> +				return -EFAULT;
> +			ivideo.video_bpp        = x.bpp;
> +			ivideo.video_width      = x.xres;
> +			ivideo.video_height     = x.yres;
> +			ivideo.video_vwidth     = x.v_xres;
> +			ivideo.video_vheight    = x.v_yres;
> +			ivideo.org_x            = x.org_x;
> +			ivideo.org_y            = x.org_y;
> +			ivideo.refresh_rate     = x.vrate;
>  			ivideo.video_linelength = ivideo.video_vwidth * (ivideo.video_bpp >> 3);
>  			switch(ivideo.video_bpp) {
>          		case 8:
> @@ -1526,34 +1539,40 @@ static int sisfb_ioctl(struct inode *ino
>  			break;
>  		}
>  	   case FBIOGET_DISPINFO:
> -		sis_dispinfo((struct ap_data *)arg);
> +		sis_dispinfo(&ap);
> +		if (copy_to_user((void *)arg, &ap, sizeof(ap)))
> +			return -EFAULT;
>  		break;
>  	   case SISFB_GET_INFO:  /* TW: New for communication with X driver */
>  	        {
> -			sisfb_info *x = (sisfb_info *)arg;
> +			sisfb_info x;
>  
> -			x->sisfb_id = SISFB_ID;
> -			x->sisfb_version = VER_MAJOR;
> -			x->sisfb_revision = VER_MINOR;
> -			x->sisfb_patchlevel = VER_LEVEL;
> -			x->chip_id = ivideo.chip_id;
> -			x->memory = ivideo.video_size / 1024;
> -			x->heapstart = ivideo.heapstart / 1024;
> -			x->fbvidmode = sisfb_mode_no;
> -			x->sisfb_caps = sisfb_caps;
> -			x->sisfb_tqlen = 512; /* yet unused */
> -			x->sisfb_pcibus = ivideo.pcibus;
> -			x->sisfb_pcislot = ivideo.pcislot;
> -			x->sisfb_pcifunc = ivideo.pcifunc;
> -			x->sisfb_lcdpdc = sisfb_detectedpdc;
> -			x->sisfb_lcda = sisfb_detectedlcda;
> +			x.sisfb_id = SISFB_ID;
> +			x.sisfb_version = VER_MAJOR;
> +			x.sisfb_revision = VER_MINOR;
> +			x.sisfb_patchlevel = VER_LEVEL;
> +			x.chip_id = ivideo.chip_id;
> +			x.memory = ivideo.video_size / 1024;
> +			x.heapstart = ivideo.heapstart / 1024;
> +			x.fbvidmode = sisfb_mode_no;
> +			x.sisfb_caps = sisfb_caps;
> +			x.sisfb_tqlen = 512; /* yet unused */
> +			x.sisfb_pcibus = ivideo.pcibus;
> +			x.sisfb_pcislot = ivideo.pcislot;
> +			x.sisfb_pcifunc = ivideo.pcifunc;
> +			x.sisfb_lcdpdc = sisfb_detectedpdc;
> +			x.sisfb_lcda = sisfb_detectedlcda;
> +			if (copy_to_user((void *)arg, &x, sizeof(x)))
> +				return -EFAULT;
>  	                break;
>  		}
>  	   case SISFB_GET_VBRSTATUS:
>  	        {
>  			unsigned long *vbrstatus = (unsigned long *) arg;
> -			if(sisfb_CheckVBRetrace()) *vbrstatus = 1;
> -			else		           *vbrstatus = 0;
> +			if(sisfb_CheckVBRetrace())  {
> +				return put_user(1UL, vbrstatus);
> +			else		           
> +				return put_user(0UL, vbrstatus);
>  		}
>  	   default:
>  		return -EINVAL;
> 
> _
> 


-- 
Thomas Winischhofer
Vienna/Austria
thomas AT winischhofer DOT net          *** http://www.winischhofer.net/
twini AT xfree86 DOT org

Re: Misc 2.5 Fixes: cp-user-sisfb

[Dipankar Sarma]

On Tue, Jun 10, 2003 at 06:31:42PM +0200, Thomas Winischhofer wrote:
> 
> Thanks, but this was already fixed in my latest version. I really should 
> commit more often...

Since 2.5 is getting close to 2.6, *now* would be a good time .... 

Thanks
Dipankar

Re: Misc 2.5 Fixes: cp-user-sisfb

[James Simmons]

> > Thanks, but this was already fixed in my latest version. I really should 
> > commit more often...
> 
> Since 2.5 is getting close to 2.6, *now* would be a good time .... 

I have several fixes and patches. Unfortunely they don't get much testing 
outside a few people. I think the best plan at this point is to apply the 
newest code to the -mm tree for testing and after several weeks try to 
push it to linus. The framebuffer stuff will always take time to sync 
mainline because if its wrong your system is borked. Its better to make 
sure its working ~100% before commiting.
 

Re: Misc 2.5 Fixes: cp-user-vicam

[Dipankar Sarma]

The patch I sent yesterday is bad, turns out I didn't enable vicam
config option while compiling. Here is a replacement patch that
actually compiles.


Fix handling of user bufs (arg), use copy_from_user.


 drivers/usb/media/vicam.c |   28 +++++++++++++++++++---------
 1 files changed, 19 insertions(+), 9 deletions(-)

diff -puN drivers/usb/media/vicam.c~cp-user-vicam drivers/usb/media/vicam.c
--- linux-2.5.70-ds/drivers/usb/media/vicam.c~cp-user-vicam	2003-06-11 16:00:42.000000000 +0530
+++ linux-2.5.70-ds-dipankar/drivers/usb/media/vicam.c	2003-06-11 16:14:06.000000000 +0530
@@ -611,15 +611,20 @@ vicam_ioctl(struct inode *inode, struct 
 
 	case VIDIOCSPICT:
 		{
-			struct video_picture *vp = (struct video_picture *) arg;
+			struct video_picture vp;
 
-			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp->depth,
-			    vp->palette);
+			if (copy_from_user(&vp, arg, sizeof (vp))) {
+				retval = -EFAULT;
+				break;
+			}
 
-			cam->gain = vp->brightness >> 8;
+			DBG("VIDIOCSPICT depth = %d, pal = %d\n", vp.depth,
+			    vp.palette);
 
-			if (vp->depth != 24
-			    || vp->palette != VIDEO_PALETTE_RGB24)
+			cam->gain = vp.brightness >> 8;
+
+			if (vp.depth != 24
+			    || vp.palette != VIDEO_PALETTE_RGB24)
 				retval = -EINVAL;
 
 			break;
@@ -652,10 +657,15 @@ vicam_ioctl(struct inode *inode, struct 
 	case VIDIOCSWIN:
 		{
 
-			struct video_window *vw = (struct video_window *) arg;
-			DBG("VIDIOCSWIN %d x %d\n", vw->width, vw->height);
+			struct video_window vw;
+
+			if (copy_from_user(&vw, arg, sizeof (vw))) {
+				retval = -EFAULT;
+				break;
+			}
+			DBG("VIDIOCSWIN %d x %d\n", vw.width, vw.height);
 
-			if ( vw->width != 320 || vw->height != 240 )
+			if ( vw.width != 320 || vw.height != 240 )
 				retval = -EFAULT;
 			
 			break;

_

Re: Misc 2.5 Fixes: cp-user-vicam

[Alan Cox]

On Mer, 2003-06-11 at 11:48, Dipankar Sarma wrote:
> The patch I sent yesterday is bad, turns out I didn't enable vicam
> config option while compiling. Here is a replacement patch that
> actually compiles.

This looks odd. 2.5 unlike 2.4 video4linux has the wrapper copy the
structures in and out

Re: Misc 2.5 Fixes: cp-user-vicam

[Dipankar Sarma]

On Wed, Jun 11, 2003 at 01:18:17PM +0100, Alan Cox wrote:
> On Mer, 2003-06-11 at 11:48, Dipankar Sarma wrote:
> > The patch I sent yesterday is bad, turns out I didn't enable vicam
> > config option while compiling. Here is a replacement patch that
> > actually compiles.
> 
> This looks odd. 2.5 unlike 2.4 video4linux has the wrapper copy the
> structures in and out

Which ioctl cmds, gets or sets ? In 2.5, it seems sets are copying
in and gets are copying the structures out as one would expect.

That said, some like VIDIOCSCHAN and VIDIOCSWIN in vicam don't 
seem to really do anything.

Thanks
Dipankar