Re: [PATCH] b43legacy: Add checking for null for ssb_get_devtypedata(dev)

On Fri, Feb 10, 2023 at 02:12:28PM +0300, Natalia Petrova wrote:

Function ssb_get_devtypedata(dev) may return null (next call
B43legacy_WARN_ON(!wl) is used for error handling, including null-value).
Therefore, a check is added before calling b43legacy_wireless_exit(),
where the argument containing this value is expected to be dereferenced.

I see that is true, however, in that case are resources leaked
due to the ieee80211_free_hw() call in b43legacy_wireless_exit()
not being made?

Moreover, aren't there also unguarded dereferences of wl:

1. In the call to b43legacy_one_core_attach(),
   which would branch to err_wireless_exit on failure.

2. In the call to schedule_work() just about the out: label.

For the record, and because it seems relevant to give contexxt,
b43legacy_probe() looks like this:

static int b43legacy_probe(struct ssb_device *dev,
                         const struct ssb_device_id *id)
{
        struct b43legacy_wl *wl;
        int err;
        int first = 0;

        wl = ssb_get_devtypedata(dev);
        if (!wl) {
                /* Probing the first core - setup common struct b43legacy_wl */
                first = 1;
                err = b43legacy_wireless_init(dev);
                if (err)
                        goto out;
                wl = ssb_get_devtypedata(dev);
                B43legacy_WARN_ON(!wl);
        }
        err = b43legacy_one_core_attach(dev, wl);
        if (err)
                goto err_wireless_exit;

        /* setup and start work to load firmware */
        INIT_WORK(&wl->firmware_load, b43legacy_request_firmware);
        schedule_work(&wl->firmware_load);

out:
        return err;

err_wireless_exit:
        if (first)
                b43legacy_wireless_exit(dev, wl);
        return err;
}