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
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,
_
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;
}
}
_
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
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 {
_
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);
_
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);
}
_
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 */
_
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:
_
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
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:
_
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;
_
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 */
_
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 )
_
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;
_
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;
_
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) {
_
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
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
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;
_
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
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