From mboxrd@z Thu Jan 1 00:00:00 1970 From: Robert Jarzmik Date: Sun, 25 Oct 2015 07:54:16 +0000 Subject: Re: [patch] mtd/docg3: off by one in doc_register_sysfs() Message-Id: <87wpubl6xj.fsf@belgarion.home> List-Id: References: <20151019102005.GF26688@mwanda> <87twpgwahr.fsf@belgarion.home> <20151024174912.GD7289@mwanda> In-Reply-To: <20151024174912.GD7289@mwanda> (Dan Carpenter's message of "Sat, 24 Oct 2015 20:49:12 +0300") MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Dan Carpenter , Brian Norris Cc: David Woodhouse , linux-mtd@lists.infradead.org, kernel-janitors@vger.kernel.org Dan Carpenter writes: > On Sat, Oct 24, 2015 at 11:49:27AM +0200, Robert Jarzmik wrote: >> Dan Carpenter writes: >> >> > Smatch found a bug in the error handling: >> > >> > drivers/mtd/devices/docg3.c:1634 doc_register_sysfs() >> > error: buffer overflow 'doc_sys_attrs' 4 <= 4 >> > >> > The problem is that if the very last device_create_file() fails, then we >> > are beyond the end of the array. Actually, any time i = 3 then there >> > is a problem. We can fix this an simplify the code at the same time by >> > moving the !ret conditions out of the for loops and using a goto >> > instead. >> >> Hi Dan, >> >> I must admit I don't see the issue here : >> - if the last device_create_file() fail, we have : >> - i = 3, ret = -Exxx >> - doc_sys_attrs[floor][0] is populated >> - doc_sys_attrs[floor][1] is populated >> - doc_sys_attrs[floor][2] is populated >> - doc_sys_attrs[floor][3] is probably NULL > > We increment "i" to 4. Ah yes, I see it now, thanks. Somehow in my brain the !ret condition in the for loop was preventing the increment ... silly. So: Acked-by: Robert Jarzmik Cheers. -- Robert