Hi Jiri,

On Mon, Feb 24, 2025 at 7:04 AM Jiri Pirko jiri@resnulli.us wrote:

Mon, Feb 24, 2025 at 10:31:27AM +0100, arkadiusz.kubalewski@intel.com wrote:

...

Hi Jiasheng, many thanks for the patch!

...

From: Jiasheng Jiang jiashengjiangcool@gmail.com Sent: Sunday, February 23, 2025 9:17 PM

When src->freq_supported is not NULL but src->freq_supported_num is 0, dst->freq_supported is equal to src->freq_supported. In this case, if the subsequent kstrdup() fails, src->freq_supported may

The src->freq_supported is not being freed in this function, you ment dst->freq_supported? But also it is not true. dst->freq_supported is being freed already, this patch adds only additional condition over it.. From kfree doc: "If @object is NULL, no operation is performed.".

...

be freed without being set to NULL, potentially leading to a use-after-free or double-free error.

kfree does not set to NULL from what I know. How would it lead to use-after-free/double-free? Why the one would use the memory after the function returns -ENOMEM?

I don't think this patch is needed or resolves anything.

I'm sure it's not needed.

After "memcpy(dst, src, sizeof(*dst))", dst->freq_supported will point to the same memory as src->freq_supported. When src->freq_supported is not NULL but src->freq_supported_num is 0, dst->freq_supported still points to the same memory as src->freq_supported. Then, if the subsequent kstrdup() fails, dst->freq_supported is freed, and src->freq_supported becomes a Dangling Pointer, potentially leading to a use-after-free or double-free error.

-Jiasheng

...

Thank you! Arkadiusz

...

Fixes: 830ead5fb0c5 ("dpll: fix pin dump crash for rebound module") Cc: stable@vger.kernel.org # v6.8+ Signed-off-by: Jiasheng Jiang jiashengjiangcool@gmail.com

---

drivers/dpll/dpll_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c index 32019dc33cca..7d147adf8455 100644 --- a/drivers/dpll/dpll_core.c +++ b/drivers/dpll/dpll_core.c @@ -475,7 +475,8 @@ static int dpll_pin_prop_dup(const struct dpll_pin_properties *src, err_panel_label: kfree(dst->board_label); err_board_label:

• kfree(dst->freq_supported);
  

• if (src->freq_supported_num)
  

•         kfree(dst->freq_supported);
  

  return -ENOMEM;

}

-- 2.25.1

Since the driver is broken in the case that src->freq_supported is not NULL but src->freq_supported_num is 0, add an assertion for it.

Fixes: 830ead5fb0c5 ("dpll: fix pin dump crash for rebound module") Cc: stable@vger.kernel.org # v6.8+ Signed-off-by: Jiasheng Jiang jiashengjiangcool@gmail.com --- Changelog:

v1 -> v2:

1. Replace the check with an assertion. --- drivers/dpll/dpll_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c index 32019dc33cca..3296776c1ebb 100644 --- a/drivers/dpll/dpll_core.c +++ b/drivers/dpll/dpll_core.c @@ -443,8 +443,9 @@ static void dpll_pin_prop_free(struct dpll_pin_properties *prop) static int dpll_pin_prop_dup(const struct dpll_pin_properties *src, struct dpll_pin_properties *dst) { + BUG_ON(src->freq_supported && !src->freq_supported_num); memcpy(dst, src, sizeof(*dst)); - if (src->freq_supported && src->freq_supported_num) { + if (src->freq_supported) { size_t freq_size = src->freq_supported_num * sizeof(*src->freq_supported); dst->freq_supported = kmemdup(src->freq_supported,

Since the driver is broken in the case that src->freq_supported is not NULL but src->freq_supported_num is 0, add an assertion for it.

Signed-off-by: Jiasheng Jiang jiashengjiangcool@gmail.com --- Changelog:

v2 -> v3:

1. Add "net-next" to the subject. 2. Remove the "Fixes" tag and "Cc: stable". 3. Replace BUG_ON with WARN_ON.

v1 -> v2:

1. Replace the check with an assertion. --- drivers/dpll/dpll_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c index 32019dc33cca..0927eddbd417 100644 --- a/drivers/dpll/dpll_core.c +++ b/drivers/dpll/dpll_core.c @@ -443,8 +443,9 @@ static void dpll_pin_prop_free(struct dpll_pin_properties *prop) static int dpll_pin_prop_dup(const struct dpll_pin_properties *src, struct dpll_pin_properties *dst) { + WARN_ON(src->freq_supported && !src->freq_supported_num); memcpy(dst, src, sizeof(*dst)); - if (src->freq_supported && src->freq_supported_num) { + if (src->freq_supported) { size_t freq_size = src->freq_supported_num * sizeof(*src->freq_supported); dst->freq_supported = kmemdup(src->freq_supported,

On 2/26/25 20:37, Jiasheng Jiang wrote:

Since the driver is broken in the case that src->freq_supported is not NULL but src->freq_supported_num is 0, add an assertion for it.

Signed-off-by: Jiasheng Jiang jiashengjiangcool@gmail.com

Changelog:

v2 -> v3:

please post next revision as a separate thread instead of in-reply-to the previous one

please also do wait a minimum of 24h prior to submitting a new revision

1. Add "net-next" to the subject.
2. Remove the "Fixes" tag and "Cc: stable".
3. Replace BUG_ON with WARN_ON.

v1 -> v2:

1. Replace the check with an assertion.

---

drivers/dpll/dpll_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/dpll/dpll_core.c b/drivers/dpll/dpll_core.c index 32019dc33cca..0927eddbd417 100644 --- a/drivers/dpll/dpll_core.c +++ b/drivers/dpll/dpll_core.c @@ -443,8 +443,9 @@ static void dpll_pin_prop_free(struct dpll_pin_properties *prop) static int dpll_pin_prop_dup(const struct dpll_pin_properties *src, struct dpll_pin_properties *dst) {

• WARN_ON(src->freq_supported && !src->freq_supported_num);

Jiri has asked for an early return too

memcpy(dst, src, sizeof(*dst));

• if (src->freq_supported && src->freq_supported_num) {

• if (src->freq_supported) { size_t freq_size = src->freq_supported_num * sizeof(*src->freq_supported); dst->freq_supported = kmemdup(src->freq_supported,