From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753005AbdDNRLm (ORCPT ); Fri, 14 Apr 2017 13:11:42 -0400 Received: from mail-pg0-f53.google.com ([74.125.83.53]:33420 "EHLO mail-pg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751258AbdDNRLj (ORCPT ); Fri, 14 Apr 2017 13:11:39 -0400 Date: Fri, 14 Apr 2017 10:11:37 -0700 From: Matthias Kaehlcke To: Axel Lin Cc: Mark Brown , Rob Herring , Liam Girdwood , linux-kernel@vger.kernel.org Subject: Re: [PATCH] regulator: vctrl: Fix out of bounds array access for vctrl->vtable Message-ID: <20170414171137.GK28657@google.com> References: <20170414025043.13680-1-axel.lin@ingics.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20170414025043.13680-1-axel.lin@ingics.com> 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 Hi Axel, El Fri, Apr 14, 2017 at 10:50:43AM +0800 Axel Lin ha dit: > Current code only allocates rdesc->n_voltages entries for vctrl->vtable. > Thus use rdesc->n_voltages instead of n_voltages in the for loop. This is intended. n_voltages is the number of voltages of the control regulator. In the first loop of the function we determine how many of these voltages are usable by the vctrl regulator (=> rdesc->n_voltages). The loop that populates vctrl->vtable iterates over n_voltages, however it skips those that are outside of the voltage range for the control regulator. > While at it, also switch to use devm_kcalloc instead of devm_kmalloc_array > + __GFP_ZERO flag and fix the argument order. This looks good to me. Thanks Matthias > Signed-off-by: Axel Lin > --- > drivers/regulator/vctrl-regulator.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/regulator/vctrl-regulator.c b/drivers/regulator/vctrl-regulator.c > index 6baadef..78de002 100644 > --- a/drivers/regulator/vctrl-regulator.c > +++ b/drivers/regulator/vctrl-regulator.c > @@ -345,9 +345,9 @@ static int vctrl_init_vtable(struct platform_device *pdev) > return -EINVAL; > } > > - vctrl->vtable = devm_kmalloc_array( > - &pdev->dev, sizeof(struct vctrl_voltage_table), > - rdesc->n_voltages, GFP_KERNEL | __GFP_ZERO); > + vctrl->vtable = devm_kcalloc(&pdev->dev, rdesc->n_voltages, > + sizeof(struct vctrl_voltage_table), > + GFP_KERNEL); > if (!vctrl->vtable) > return -ENOMEM; > > @@ -371,7 +371,7 @@ static int vctrl_init_vtable(struct platform_device *pdev) > NULL); > > /* pre-calculate OVP-safe downward transitions */ > - for (i = n_voltages - 1; i > 0; i--) { > + for (i = rdesc->n_voltages - 1; i > 0; i--) { > int j; > int ovp_min_uV = (vctrl->vtable[i].out * > (100 - vctrl->ovp_threshold)) / 100;