[LU-6520] Potential null pointer deref in mdt_stack_init and mdt_quota_init

• Status: Open
• Project: Lustre
• Affects Version/s: Lustre 2.15.0
• Type: Bug
• Priority: Minor
• Reporter: Oleg Drokin
• Assignee: WC Triage
• Resolution: Unresolved
• Severity: 3

Description

smatch highlighted problematic code in mdt_stack_init and mdt_quota_init

        lcfg = lustre_cfg_new(LCFG_SETUP, bufs);
        if (lcfg == NULL)
                GOTO(class_detach, rc = -ENOMEM);
...
class_detach:
        if (rc)
                class_detach(obd, lcfg);
lcfg_cleanup:
        lustre_cfg_free(lcfg);

note that while lustre_cfs_Free is basically kfree, which is ok to work with NULL pointers, in reality it does

static inline void lustre_cfg_free(struct lustre_cfg *lcfg)
{
#ifdef __KERNEL__
        OBD_FREE(lcfg, lustre_cfg_len(lcfg->lcfg_bufcount, lcfg->lcfg_buflens));

which makes it not ok.

Comments

[Ulka Vaze (Inactive)]

Hi,
lustre_cfg_free code is as below-

static inline void lustre_cfg_free(struct lustre_cfg *lcfg)
{
#ifdef __KERNEL__
         OBD_FREE(lcfg, lustre_cfg_len(lcfg->lcfg_bufcount, lcfg->lcfg_buflens));
#else /* ! __KERNEL__ */
    free(lcfg);
#endif /* __KERNEL__ */
}

OBD_FREE calls OBD_FREE_PRE(ptr, size, "kfreed");
OBD_FREE_PRE asserts the pointer.

#define OBD_FREE_PRE(ptr, size, name)                                   \
        LASSERT(ptr);                                                   \
        obd_memory_sub(size);                                           \
        CDEBUG(D_MALLOC, name " '" #ptr "': %d at %p.\n",               \
               (int)(size), ptr);                                       \
        POISON(ptr, 0x5a, size)

So this LASSERT will fail if lcfg pointer is NULL.

I think this way we will get logs why memory allocation failed.
And it is not causing dereferencing of NULL pointer.

Having said that we can add extra check lcfg != NULL in lustre_cfg_free. But this will not give us reason of failure.

-Ulka

[Oleg Drokin]

The this is - if we add the check, then we'll avoid the crash and whatever the failure is, memory allocator will print us something in dmesg.