From 4b030d4288a569d6bdeca884d7f102d951f097f2 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Wed, 4 Aug 2010 23:38:06 +0000 Subject: isdn: fix information leak The main motivation of this patch changing strcpy() to strlcpy(). We strcpy() to copy a 48 byte buffers into a 49 byte buffers. So at best the last byte has leaked information, or maybe there is an overflow? Anyway, this patch closes the information leaks by zeroing the memory and the calls to strlcpy() prevent overflows. Signed-off-by: Dan Carpenter Signed-off-by: David S. Miller --- drivers/isdn/sc/ioctl.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/sc/ioctl.c b/drivers/isdn/sc/ioctl.c index 43c5dc3516e..4cfdbe08ffd 100644 --- a/drivers/isdn/sc/ioctl.c +++ b/drivers/isdn/sc/ioctl.c @@ -174,7 +174,7 @@ int sc_ioctl(int card, scs_ioctl *data) pr_debug("%s: SCIOGETSPID: ioctl received\n", sc_adapter[card]->devicename); - spid = kmalloc(SCIOC_SPIDSIZE, GFP_KERNEL); + spid = kzalloc(SCIOC_SPIDSIZE, GFP_KERNEL); if (!spid) { kfree(rcvmsg); return -ENOMEM; @@ -194,7 +194,7 @@ int sc_ioctl(int card, scs_ioctl *data) kfree(rcvmsg); return status; } - strcpy(spid, rcvmsg->msg_data.byte_array); + strlcpy(spid, rcvmsg->msg_data.byte_array, SCIOC_SPIDSIZE); /* * Package the switch type and send to user space @@ -266,12 +266,12 @@ int sc_ioctl(int card, scs_ioctl *data) return status; } - dn = kmalloc(SCIOC_DNSIZE, GFP_KERNEL); + dn = kzalloc(SCIOC_DNSIZE, GFP_KERNEL); if (!dn) { kfree(rcvmsg); return -ENOMEM; } - strcpy(dn, rcvmsg->msg_data.byte_array); + strlcpy(dn, rcvmsg->msg_data.byte_array, SCIOC_DNSIZE); kfree(rcvmsg); /* @@ -337,7 +337,7 @@ int sc_ioctl(int card, scs_ioctl *data) pr_debug("%s: SCIOSTAT: ioctl received\n", sc_adapter[card]->devicename); - bi = kmalloc (sizeof(boardInfo), GFP_KERNEL); + bi = kzalloc(sizeof(boardInfo), GFP_KERNEL); if (!bi) { kfree(rcvmsg); return -ENOMEM; -- cgit v1.2.3 From 7e27a0aeb98d53539bdc38384eee899d6db62617 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 5 Aug 2010 22:23:23 +0000 Subject: isdn: gigaset: add missing unlock We should unlock here. This is the only place where we return from the function with the lock held. The caller isn't expecting it. Signed-off-by: Dan Carpenter Signed-off-by: David S. Miller --- drivers/isdn/gigaset/capi.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/isdn') diff --git a/drivers/isdn/gigaset/capi.c b/drivers/isdn/gigaset/capi.c index e5ea344a551..bcc174e4f3b 100644 --- a/drivers/isdn/gigaset/capi.c +++ b/drivers/isdn/gigaset/capi.c @@ -1052,6 +1052,7 @@ static inline void remove_appl_from_channel(struct bc_state *bcs, do { if (bcap->bcnext == ap) { bcap->bcnext = bcap->bcnext->bcnext; + spin_unlock_irqrestore(&bcs->aplock, flags); return; } bcap = bcap->bcnext; -- cgit v1.2.3 From 8bcfbd0af0f8ee50033091e75ab3d6b6e7fa8867 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 5 Aug 2010 22:21:26 +0000 Subject: isdn: gigaset: use after free I moved the kfree(cb) below the dereferences. Signed-off-by: Dan Carpenter Signed-off-by: David S. Miller --- drivers/isdn/gigaset/bas-gigaset.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/isdn') diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c index 0ded3640b92..707d9c94cf9 100644 --- a/drivers/isdn/gigaset/bas-gigaset.c +++ b/drivers/isdn/gigaset/bas-gigaset.c @@ -1914,11 +1914,13 @@ static int gigaset_write_cmd(struct cardstate *cs, struct cmdbuf_t *cb) * The next command will reopen the AT channel automatically. */ if (cb->len == 3 && !memcmp(cb->buf, "+++", 3)) { - kfree(cb); rc = req_submit(cs->bcs, HD_CLOSE_ATCHANNEL, 0, BAS_TIMEOUT); if (cb->wake_tasklet) tasklet_schedule(cb->wake_tasklet); - return rc < 0 ? rc : cb->len; + if (!rc) + rc = cb->len; + kfree(cb); + return rc; } spin_lock_irqsave(&cs->cmdlock, flags); -- cgit v1.2.3