Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754159AbdHXRZP (ORCPT ); Thu, 24 Aug 2017 13:25:15 -0400 Received: from mail-pg0-f51.google.com ([74.125.83.51]:35083 "EHLO mail-pg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754139AbdHXRZN (ORCPT ); Thu, 24 Aug 2017 13:25:13 -0400 Date: Thu, 24 Aug 2017 10:25:09 -0700 From: Bjorn Andersson To: Rob Herring Cc: Frank Rowand , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] of/device: Fix of_device_get_modalias() buffer handling Message-ID: <20170824172509.GE29306@minitux> References: <20170824010352.9085-1-bjorn.andersson@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.8.3 (2017-05-23) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1531 Lines: 48 On Thu 24 Aug 09:51 PDT 2017, Rob Herring wrote: > On Wed, Aug 23, 2017 at 8:03 PM, Bjorn Andersson > wrote: [..] > > @@ -206,12 +207,16 @@ static ssize_t of_device_get_modalias(struct device *dev, char *str, ssize_t len > > /* Name & Type */ > > csize = snprintf(str, len, "of:N%sT%s", dev->of_node->name, > > dev->of_node->type); > > + tsize = csize; > > len -= csize; > > - str += csize; > > + if (str) > > + str += csize; > > > > of_property_for_each_string(dev->of_node, "compatible", p, compat) { > > - if (strlen(compat) + 2 > len) > > - break; > > + csize = strlen(compat) + 1; > > + tsize += csize; > > + if (csize > len) > > + continue; > > > > csize = snprintf(str, len, "C%s", compat); > > We could just use the snprintf to give us the length. Something like > this following the snprintf: > > tsize +=csize; > if (csize > len) { > if (len) { > str[0] = '\0'; > len = 0; > } > continue; > } > > You'd need to prevent len from going negative up above too. It ends up > being more lines but you save a strlen call. So perhaps it's fine as > is, but since I've written it already throwing it out there... > Indeed, I did attempt a few variants of this as well, but ended up taking the cost of an extra strlen() to keep the complexity of the code down. Regards, Bjorn