Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751503AbdITAVA (ORCPT ); Tue, 19 Sep 2017 20:21:00 -0400 Received: from mail-pg0-f65.google.com ([74.125.83.65]:36415 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751074AbdITAU6 (ORCPT ); Tue, 19 Sep 2017 20:20:58 -0400 X-Google-Smtp-Source: AOwi7QAYzRzGsFvKWpnxQB+dpMe3rUbK1YkIMEdAq8c9Mfl0H1H/iiqEyjtik9T87IEk34QZHLV0Cw== Subject: Re: [PATCH] of: overlay: Fix uninitialized vars in dup_and_fixup_symbol_prop() To: Geert Uytterhoeven References: <1505039164-25468-1-git-send-email-geert@linux-m68k.org> <59C16197.4040403@gmail.com> Cc: Pantelis Antoniou , Rob Herring , Grant Likely , Arnd Bergmann , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" From: Frank Rowand Message-ID: <59C1B465.1010209@gmail.com> Date: Tue, 19 Sep 2017 17:20:53 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2245 Lines: 58 On 09/19/17 13:16, Geert Uytterhoeven wrote: > Hi Frank, > > On Tue, Sep 19, 2017 at 8:27 PM, Frank Rowand wrote: >> On 09/10/17 03:26, Geert Uytterhoeven wrote: >>> With gcc 4.1.2: >>> >>> drivers/of/overlay.c: In function ‘dup_and_fixup_symbol_prop’: >>> drivers/of/overlay.c:108: warning: ‘overlay_name_len’ may be used uninitialized in this function >>> drivers/of/overlay.c:100: warning: ‘ovinfo’ may be used uninitialized in this function >>> >>> Indeed, if ov->count == 0, both variables are uninitialized, which may >>> lead to a crash when dereferencing ovinfo later. >>> >>> Currently this is a false positive, as the sole creator of of_overlay >>> structures (of_build_overlay_info(), introduced in commit >>> 7518b5890d8ac366 ("of/overlay: Introduce DT overlay support") checks for >>> this. >>> >>> To prevent future issues, add a check for a zero ov->count to >>> dup_and_fixup_symbol_prop(). Note that this does not get rid of the >>> actual compiler warning. >>> >>> Fixes: d1651b03c2df75db ("of: overlay: add overlay symbols to live device tree") >>> Signed-off-by: Geert Uytterhoeven >>> --- >>> drivers/of/overlay.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c >>> index 8ecfee31ab6d3874..ebe19e0f8e4d1f4b 100644 >>> --- a/drivers/of/overlay.c >>> +++ b/drivers/of/overlay.c >>> @@ -108,7 +108,7 @@ static struct property *dup_and_fixup_symbol_prop(struct of_overlay *ov, >>> int overlay_name_len; >>> int target_path_len; >>> >>> - if (!prop->value) >>> + if (!ov->count || !prop->value) >>> return NULL; >>> symbol_path = prop->value; >>> >> >> I did not see this patch due to an overzealous spam filter. I noticed it >> when Rob replied with his applied email. >> >> This check is not needed to prevent accessing overlay_name_len and ovinfo >> when ov->count == 0. That is already prevented by: >> >> if (k >= ov->count) >> goto err_free; >> >> because k will be zero and ov->count will be zero. > > Thank you, I stand corrected. No problem. It's not real obvious, you really need to stop and ponder.