Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755135Ab3FGI7U (ORCPT ); Fri, 7 Jun 2013 04:59:20 -0400 Received: from mail-pa0-f48.google.com ([209.85.220.48]:35584 "EHLO mail-pa0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754086Ab3FGI7Q (ORCPT ); Fri, 7 Jun 2013 04:59:16 -0400 Date: Fri, 7 Jun 2013 16:59:05 +0800 From: Adam Lee To: =?iso-8859-1?Q?Bj=F8rn?= Mork Cc: linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org, ibm-acpi-devel@lists.sourceforge.net, Matthew Garrett , Henrique de Moraes Holschuh , Alex Hung Subject: Re: [PATCH 1/3] thinkpad_acpi: return -NODEV while operating uninitialized LEDs Message-ID: <20130607085905.GA7325@adam-laptop> Mail-Followup-To: =?iso-8859-1?Q?Bj=F8rn?= Mork , linux-kernel@vger.kernel.org, platform-driver-x86@vger.kernel.org, ibm-acpi-devel@lists.sourceforge.net, Matthew Garrett , Henrique de Moraes Holschuh , Alex Hung References: <1370593209-21358-1-git-send-email-adam.lee@canonical.com> <1370593209-21358-2-git-send-email-adam.lee@canonical.com> <87vc5qxpwh.fsf@nemi.mork.no> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <87vc5qxpwh.fsf@nemi.mork.no> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1120 Lines: 35 On Fri, Jun 07, 2013 at 10:53:50AM +0200, Bj?rn Mork wrote: > Adam Lee writes: > > > --- a/drivers/platform/x86/thinkpad_acpi.c > > +++ b/drivers/platform/x86/thinkpad_acpi.c > > @@ -5401,9 +5401,12 @@ static int led_write(char *buf) > > return -ENODEV; > > > > while ((cmd = next_cmd(&buf))) { > > - if (sscanf(cmd, "%d", &led) != 1 || led < 0 || led > 15) > > + if (sscanf(cmd, "%d", &led) != 1) > > return -EINVAL; > > > > + if (!tpacpi_leds[led].led) > > + return -ENODEV; > > This looks risky. Why did you remove the index sanity check? What will > happen now if the input is e.g "-1" or "42"? > > BTW, the magic number 15 should probably be (TPACPI_LED_NUMLEDS - 1) > instead. Oh, good point! I was thinking led_init() already checked the index. Will submit patch v2, thanks. -- Regards, Adam Lee Hardware Enablement -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/