re: cxl/core: Split decoder setup into alloc + add

* re: cxl/core: Split decoder setup into alloc + add
@ 2021-08-29 18:27 Colin Ian King
  0 siblings, 0 replies; only message in thread
From: Colin Ian King @ 2021-08-29 18:27 UTC (permalink / raw)
  To: Dan Williams
  Cc: Alison Schofield, Vishal Verma, Ira Weiny, Ben Widawsky, linux-cxl

Hi,

Static analysis on linux-next with Coverity had detected an issue with
the following commit:

commit b7ca54b625514464bac2db59b754e95c49b66fb5
Author: Dan Williams <dan.j.williams@intel.com>
Date:   Tue Aug 24 09:07:56 2021 -0700

    cxl/core: Split decoder setup into alloc + add

The analysis is as follows:

494 int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
495                         int *target_map)
496{
497        struct cxl_port *port = to_cxl_port(cxld->dev.parent);

   1. var_decl: Declaring variable dev without initializer.

498        struct device *dev;
499        int rc = 0, i;
500

   2. Condition !cxld, taking false branch.

501        if (!cxld)
502                return -EINVAL;
503

   3. Condition IS_ERR(cxld), taking false branch.

504        if (IS_ERR(cxld))
505                return PTR_ERR(cxld);
506

   4. Condition cxld->interleave_ways < 1, taking true branch.

507        if (cxld->interleave_ways < 1) {
508                rc = -EINVAL;

   5. Jumping to label err.

509                goto err;
510        }
511
512        device_lock(&port->dev);
513        if (list_empty(&port->dports))
514                rc = -EINVAL;
515
516        for (i = 0; rc == 0 && target_map && i < cxld->nr_targets; i++) {
517                struct cxl_dport *dport = find_dport(port,
target_map[i]);
518
519                if (!dport) {
520                        rc = -ENXIO;
521                        break;
522                }
523                dev_dbg(host, "%s: target: %d\n",
dev_name(dport->dport), i);
524                cxld->target[i] = dport;
525        }
526        device_unlock(&port->dev);
527        if (rc)
528                goto err;
529
530        dev = &cxld->dev;
531        rc = dev_set_name(dev, "decoder%d.%d", port->id, cxld->id);
532        if (rc)
533                goto err;
534
535        rc = device_add(dev);
536        if (rc)
537                goto err;
538
539        return devm_add_action_or_reset(host, unregister_cxl_dev, dev);
540 err:

   Uninitialized pointer read (UNINIT)

   6. uninit_use_in_call: Using uninitialized value dev when calling
put_device.

541        put_device(dev);
542        return rc;
543 }

There maybe more than one incorrect jump to the error exit path label
'err'. As it stands, the put_device(dev) is occurring on at least one
error jump to err.

Colin

^ permalink raw reply	[flat|nested] only message in thread