From mboxrd@z Thu Jan 1 00:00:00 1970 From: Changli Gao Subject: Re: [patch] isdn: fix information leak Date: Thu, 5 Aug 2010 21:59:52 +0800 Message-ID: References: <20100805093806.GF9031@bicker> <20100805101938.GH9031@bicker> <20100805113721.GI9031@bicker> <20100805134910.GJ9031@bicker> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Karsten Keil , netdev@vger.kernel.org, kernel-janitors@vger.kernel.org To: Dan Carpenter Return-path: Received: from mail-ww0-f44.google.com ([74.125.82.44]:35809 "EHLO mail-ww0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1759092Ab0HEOAN convert rfc822-to-8bit (ORCPT ); Thu, 5 Aug 2010 10:00:13 -0400 In-Reply-To: <20100805134910.GJ9031@bicker> Sender: netdev-owner@vger.kernel.org List-ID: On Thu, Aug 5, 2010 at 9:55 PM, Dan Carpenter wrote= : > First of all, I'm happy that you're reviewing these patches. =A0A lot= of > learner, often untested, patches go through kernel-janitors and the > entire purpose of the list is to check each other's work. > > Here is what I meant by an information leak. =A0The original code did= : > =A0 =A0 =A0 =A0spid =3D kmalloc(SCIOC_SPIDSIZE, GFP_KERNEL); > =A0 =A0 =A0 =A0... > =A0 =A0 =A0 =A0strcpy(spid, rcvmsg->msg_data.byte_array); > =A0 =A0 =A0 =A0... > =A0 =A0 =A0 =A0if (copy_to_user(data->dataptr, spid, SCIOC_SPIDSIZE))= { > > When you allocate memory with kmalloc() it doesn't clear that memory.= =A0It > could have anything in there including passwords etc. =A0If there is = a > short string in "rcvmsg->msg_data.byte_array" like "123" then the > strcpy() puts that in the first 4 bytes of "spid", but the rest of th= e > buffer is still full of passwords. > > The copy_to_user() sends it to the user, and normally the user only r= eads > as far as the terminator, but if they read past that then they would = see > all the passwords etc that were saved there. > > This function is in the ioctl() probably only root has open() permiss= ion > on the device so it's not a big deal because root can already find yo= ur > password. =A0But I didn't dig that far. =A0It's just simpler to zero = out the > memory instead of worrying about if it really is exploitable or not. > > Also it's quite possible that the strcpy can't overflow. =A0But it's = weird > for the code to leave space for the terminator and then not use it. =A0= In > the end, I decided to do the cautious thing and be done with it. > Thanks, Dan. I got it. --=20 Regards, Changli Gao(xiaosuo@gmail.com)