Latent dead-code issues found while auditing kernel-open/nvidia

[VaggelisGian]

Title: Latent dead-code issues found while auditing kernel-open/nvidia

While reading through the Tegra-side helper code in kernel-open/nvidia I ran
into three small defects. All of them sit inside functions whose call chains
are currently unreachable in-tree, so none of them is a functional bug today,
which is why this is a forum post rather than an issue or a patch. Posting for
awareness since these paths look like candidates to be wired up again in
future releases.

1. Unchecked tegra_bpmp_get() and missing put in nv_imp_enable_disable_rfl

kernel-open/nvidia/nv-imp.c:181

struct tegra_bpmp *bpmp = tegra_bpmp_get(nvl->dev);

tegra_bpmp_get() can return ERR_PTR(-ENODEV) when no BPMP handle is available,
but the return value is used directly in tegra_bpmp_transfer() at line 195
without an IS_ERR() check. The reference taken by tegra_bpmp_get() is also
never released; there is no tegra_bpmp_put() anywhere in the function. The
same get/put imbalance was recently fixed in nv-bpmp.c ("nvidia: Release the
BPMP device reference after sending an MRQ"), this site just was not part of
that path.

The caller chain is osTegraSocEnableDisableRfl() in
src/nvidia/arch/nvalloc/unix/src/os.c:5768, which currently has zero callers
in-tree (declaration only in generated g_os_nvoc.h).

Suggested fix when this gets wired up: IS_ERR()/PTR_ERR() guard after the
get, and a tegra_bpmp_put() after tegra_bpmp_transfer(), matching the shape
now used in nv-bpmp.c.

2. Wrong status code and leaked adapter reference in nv_i2c_bus_status

kernel-open/nvidia/nv-i2c.c:471-513

Two problems in the same function:

a) Line 497 returns NULL from an NV_STATUS function:

i2c_adapter = i2c_get_adapter(linuxI2CSwPort);
if (i2c_adapter == NULL)
{
    ...
    return NULL;
}

NULL converts to 0, which is NV_OK, so the documented failure case reports
success to the RM layer. It should return an error such as
NV_ERR_INVALID_ARGUMENT or NV_ERR_GENERIC.

b) The ret < 0 path at lines 502-506 returns NV_ERR_GENERIC without calling
i2c_put_adapter(), leaking the reference taken at line 492. Only the success
path reaches the put at line 507.

Caller chain: osTegraI2CGetBusState() in os.c:3450, which likewise has zero
callers in-tree today.

3. Backlight name handling overwrites a heap pointer with a device tree pointer

kernel-open/nvidia/nv-dsi-parse-panel-props.c:437-448

NV_KMALLOC(dsi->bl_name, sizeof(u8) * bl_name_len);
if (!of_property_read_string(np_dsi_panel,
    "nvidia,dsi-bl-name",
    (const char **)&dsi->bl_name)) {
} else {
    ...
    NV_KFREE(dsi->bl_name, sizeof(u8) * bl_name_len);
}

On the success path, of_property_read_string() stores a pointer into the
flattened device tree blob over the freshly allocated dsi->bl_name buffer, so
the kmalloc result leaks and bl_name ends up pointing into DT memory. The
cleanup at lines 1003-1004 then calls NV_KFREE() on that DT pointer, which
would pass non-slab memory to kfree(). On the failure path the free happens
but the caller cannot distinguish the two states afterwards.

The enclosing function nv_dsi_parse_panel_props() currently has zero in-tree
call sites, so nothing reaches this today.

Suggested direction: read into a local const char * first and memcpy into the
allocated buffer, keeping ownership of dsi->bl_name heap-backed.

Happy to provide more detail on any of these, or to send patches if/when the
call sites become live.

[VaggelisGian]

One more item for this list, found in the same audit.

4. Wrong format specifier when the BPMP handle cannot be obtained

kernel-open/nvidia/nv-clk.c:814-816 and 861-862

bpmp = tegra_bpmp_get(nvl->dev);
if (IS_ERR(bpmp))
{
    nv_printf(NV_DBG_ERRORS,
              "NVRM:  Error getting bpmp struct: %s\n",
              PTR_ERR(bpmp));
    return NV_ERR_GENERIC;
}

PTR_ERR() returns a long errno value but it is formatted with %s, so
vsnprintf walks the negative error code as a char pointer. On kernels
before v5.2 (this driver supports 4.15+) lib/vsprintf.c has no bad
pointer protection, so executing the print faults; from v5.2 onward
check_pointer_msg() turns it into a harmless "(efault)" instead of the
errno. The same file uses the correct form at line 753:
"clk_get_parent failed with error: %ld".

Like the items above this one is currently unreachable in-tree: both
functions (the DisplayPort UPHY PLL init/deinit hooks) have no callers
in the published tree. The fix whenever these paths get exercised is
just %s -> %ld.