lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230917091225.6350-1-brgl@bgdev.pl>
Date:   Sun, 17 Sep 2023 11:12:23 +0200
From:   Bartosz Golaszewski <brgl@...ev.pl>
To:     Linus Walleij <linus.walleij@...aro.org>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Kent Gibson <warthog618@...il.com>,
        Alexey Dobriyan <adobriyan@...il.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        akpm@...ux-foundation.org
Cc:     linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: [PATCH v3 0/2] gpio: sim: improve the usage of __free()

From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>

Hi Linus et al,

I'm sorry for the noise but v2 was incorrect, this is a fixed version.

As discussed here's an improved fix for the invalid usage of __free() in
gpio-sim. I based it on your "maybe-sane" suggestion but unfortunately it
missed a couple details that make it impossible to avoid a conditional
initialization of the managed pointer without repeating the call to
fwnode_create_software_node().

What we're doing here is: we're creating the string array for the standard
"gpio-line-names" device property. It can look like this:

    { "foo", "bar", "baz" }

In which case lines 0, 1 and 2 are named but it can also look like this:

    { "foo", NULL, NULL, "bar", NULL, "baz" }

Where only lines 0, 3 and 5 have assigned names.

So the `has_line_names` boolean set when encountering the first line with a
name is there for a reason, namely: it's possible that only the line at
offset 0 will have a name, leaving max_offset at 0 but we still need to
create an array of size 1 in this case.

If the array is created and filled, then it needs to live until a deep
copy is completed in fwnode_create_software_node() so it has to be defined
at the top of the function.

I think this still results in clearer code then if we called
`return fwnode_create_software_node();` twice with the same arguments.

I also changed the naming to reflect the purpose of the array: it can be
sparse so it's not really "number of lines", it's the "size of the array
holding the names". The array can be of size 10 but we can only have 3
named lines.

To atone for the above, I've added a second patch which changes the other
instance of __free() in this driver to be initialized in place.

If this is alright for you, please consider applying it directly to your
tree for v6.6-rc2.

Best regards,
Bartosz Golaszewski

v2 -> v3:
- restore the offset out-of-bounds checks

v1 -> v2:
- split the line name setting into two parts
- add a patch improving the second instance of using __free()

Bartosz Golaszewski (2):
  gpio: sim: fix an invalid __free() usage
  gpio: sim: initialize a managed pointer when declaring it

 drivers/gpio/gpio-sim.c | 68 +++++++++++++++++++----------------------
 1 file changed, 32 insertions(+), 36 deletions(-)

-- 
2.39.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ