netdev - Re: [patch] isdn: fix a wrapping bug in isdn_ppp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1349864358.2386.27.camel@joe-AO722>
Date:	Wed, 10 Oct 2012 03:19:18 -0700
From:	Joe Perches <joe@...ches.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
Cc:	Karsten Keil <isdn@...ux-pingi.de>,
	"David S. Miller" <davem@...emloft.net>,
	Masanari Iida <standby24x7@...il.com>, netdev@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [patch] isdn: fix a wrapping bug in isdn_ppp_ioctl()

On Wed, 2012-10-10 at 12:42 +0300, Dan Carpenter wrote:
> "protos" is an array of unsigned longs and "i" is the number of bits in
> an unsigned long so we need to use 1UL as well to prevent the shift
> from wrapping around.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@...cle.com>
> 
> diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
> index a1e7601..69b5b58 100644
> --- a/drivers/isdn/i4l/isdn_ppp.c
> +++ b/drivers/isdn/i4l/isdn_ppp.c
> @@ -595,7 +595,7 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
>  			j = ipc->num / (sizeof(long) * 8);
>  			i = ipc->num % (sizeof(long) * 8);
>  			if (j < 8)
> -				protos[j] |= (0x1 << i);
> +				protos[j] |= (1UL << i);

This looks like a bitmap and probably it should use
DECLARE_BITMAP and set_bit.
Also, it looks like the set_arg size is wrong and is
a stack leak.

Maybe:

 drivers/isdn/i4l/isdn_ppp.c |   13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index a1e7601..6438d0c 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -471,7 +471,7 @@ int
 isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
 {
 	unsigned long val;
-	int r, i, j;
+	int r;
 	struct ippp_struct *is;
 	isdn_net_local *lp;
 	struct isdn_ppp_comp_data data;
@@ -589,16 +589,15 @@ isdn_ppp_ioctl(int min, struct file *file, unsigned int cmd, unsigned long arg)
 		break;
 	case PPPIOCGCOMPRESSORS:
 	{
-		unsigned long protos[8] = {0,};
+		DECLARE_BITMAP(protos, BITS_PER_LONG * 8) = { 0, };
 		struct isdn_ppp_compressor *ipc = ipc_head;
+
 		while (ipc) {
-			j = ipc->num / (sizeof(long) * 8);
-			i = ipc->num % (sizeof(long) * 8);
-			if (j < 8)
-				protos[j] |= (0x1 << i);
+			if (ipc->num < BITS_PER_LONG * 8)
+				set_bit(ipc->num, protos);
 			ipc = ipc->next;
 		}
-		if ((r = set_arg(argp, protos, 8 * sizeof(long))))
+		if ((r = set_arg(argp, protos, sizeof(protos))))
 			return r;
 	}
 	break;


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html