From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org
Subject: Re: [cxl-cxl:pending 39/40] drivers/cxl/core/bus.c:501 devm_cxl_add_decoder() warn: variable dereferenced before check 'cxld' (see line 497)
Date: Thu, 26 Aug 2021 15:50:21 +0300 [thread overview]
Message-ID: <20210826125020.GA7722@kadam> (raw)
In-Reply-To: <202108250714.GDy2jUg2-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 5055 bytes --]
On Wed, Aug 25, 2021 at 10:12:32AM +0300, Dan Carpenter wrote:
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 494 int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
> 574d46ed53b527 drivers/cxl/core/bus.c Dan Williams 2021-08-24 495 int *target_map)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 496 {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 @497 struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> ^^^^^^^^^^^^^^^^
> Dereference
>
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 498 struct device *dev;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 499 int rc = 0, i;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 500
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 @501 if (!cxld)
> ^^^^^
> Checked too late.
>
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 502 return -EINVAL;
> 574d46ed53b527 drivers/cxl/core/bus.c Dan Williams 2021-08-24 503
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 504 if (IS_ERR(cxld))
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 505 return PTR_ERR(cxld);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 506
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 507 if (cxld->interleave_ways < 1) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 508 rc = -EINVAL;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 509 goto err;
>
> "dev" not initialized at this point.
>
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 510 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 511
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 512 device_lock(&port->dev);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 513 if (list_empty(&port->dports))
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 514 rc = -EINVAL;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 515
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 516 for (i = 0; rc == 0 && target_map && i < cxld->nr_targets; i++) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 517 struct cxl_dport *dport = find_dport(port, target_map[i]);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 518
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 519 if (!dport) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 520 rc = -ENXIO;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 521 break;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 522 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 523 dev_dbg(host, "%s: target: %d\n", dev_name(dport->dport), i);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 524 cxld->target[i] = dport;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 525 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 526 device_unlock(&port->dev);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 527 if (rc)
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 528 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 529
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 530 dev = &cxld->dev;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 531 rc = dev_set_name(dev, "decoder%d.%d", port->id, cxld->id);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 532 if (rc)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 533 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 534
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 535 rc = device_add(dev);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 536 if (rc)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 537 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 538
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 539 return devm_add_action_or_reset(host, unregister_cxl_dev, dev);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 540 err:
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 @541 put_device(dev);
>
> Should be:
>
> put_device(&cxld->dev);
>
> But it feels like a layering violation to drop a reference that was
> aquired by the caller.
This code hit linux-next yesterday so I reviewed it in context. The
put_device() should just be removed. It leads to a use after free.
regards,
dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild-all@lists.01.org
Subject: Re: [cxl-cxl:pending 39/40] drivers/cxl/core/bus.c:501 devm_cxl_add_decoder() warn: variable dereferenced before check 'cxld' (see line 497)
Date: Thu, 26 Aug 2021 15:50:21 +0300 [thread overview]
Message-ID: <20210826125020.GA7722@kadam> (raw)
In-Reply-To: <202108250714.GDy2jUg2-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 5055 bytes --]
On Wed, Aug 25, 2021 at 10:12:32AM +0300, Dan Carpenter wrote:
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 494 int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
> 574d46ed53b527 drivers/cxl/core/bus.c Dan Williams 2021-08-24 495 int *target_map)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 496 {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 @497 struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> ^^^^^^^^^^^^^^^^
> Dereference
>
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 498 struct device *dev;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 499 int rc = 0, i;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 500
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 @501 if (!cxld)
> ^^^^^
> Checked too late.
>
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 502 return -EINVAL;
> 574d46ed53b527 drivers/cxl/core/bus.c Dan Williams 2021-08-24 503
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 504 if (IS_ERR(cxld))
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 505 return PTR_ERR(cxld);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 506
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 507 if (cxld->interleave_ways < 1) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 508 rc = -EINVAL;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 509 goto err;
>
> "dev" not initialized at this point.
>
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 510 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 511
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 512 device_lock(&port->dev);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 513 if (list_empty(&port->dports))
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 514 rc = -EINVAL;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 515
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 516 for (i = 0; rc == 0 && target_map && i < cxld->nr_targets; i++) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 517 struct cxl_dport *dport = find_dport(port, target_map[i]);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 518
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 519 if (!dport) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 520 rc = -ENXIO;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 521 break;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 522 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 523 dev_dbg(host, "%s: target: %d\n", dev_name(dport->dport), i);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 524 cxld->target[i] = dport;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 525 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 526 device_unlock(&port->dev);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 527 if (rc)
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 528 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 529
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 530 dev = &cxld->dev;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 531 rc = dev_set_name(dev, "decoder%d.%d", port->id, cxld->id);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 532 if (rc)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 533 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 534
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 535 rc = device_add(dev);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 536 if (rc)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 537 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 538
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 539 return devm_add_action_or_reset(host, unregister_cxl_dev, dev);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 540 err:
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 @541 put_device(dev);
>
> Should be:
>
> put_device(&cxld->dev);
>
> But it feels like a layering violation to drop a reference that was
> aquired by the caller.
This code hit linux-next yesterday so I reviewed it in context. The
put_device() should just be removed. It leads to a use after free.
regards,
dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: kbuild@lists.01.org, Dan Williams <dan.j.williams@intel.com>
Cc: lkp@intel.com, kbuild-all@lists.01.org,
Alison Schofield <alison.schofield@intel.com>,
Vishal Verma <vishal.l.verma@intel.com>,
Ira Weiny <ira.weiny@intel.com>,
Ben Widawsky <ben.widawsky@intel.com>,
linux-kernel@vger.kernel.org
Subject: Re: [cxl-cxl:pending 39/40] drivers/cxl/core/bus.c:501 devm_cxl_add_decoder() warn: variable dereferenced before check 'cxld' (see line 497)
Date: Thu, 26 Aug 2021 15:50:21 +0300 [thread overview]
Message-ID: <20210826125020.GA7722@kadam> (raw)
In-Reply-To: <202108250714.GDy2jUg2-lkp@intel.com>
On Wed, Aug 25, 2021 at 10:12:32AM +0300, Dan Carpenter wrote:
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 494 int devm_cxl_add_decoder(struct device *host, struct cxl_decoder *cxld,
> 574d46ed53b527 drivers/cxl/core/bus.c Dan Williams 2021-08-24 495 int *target_map)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 496 {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 @497 struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> ^^^^^^^^^^^^^^^^
> Dereference
>
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 498 struct device *dev;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 499 int rc = 0, i;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 500
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 @501 if (!cxld)
> ^^^^^
> Checked too late.
>
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 502 return -EINVAL;
> 574d46ed53b527 drivers/cxl/core/bus.c Dan Williams 2021-08-24 503
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 504 if (IS_ERR(cxld))
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 505 return PTR_ERR(cxld);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 506
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 507 if (cxld->interleave_ways < 1) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 508 rc = -EINVAL;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 509 goto err;
>
> "dev" not initialized at this point.
>
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 510 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 511
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 512 device_lock(&port->dev);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 513 if (list_empty(&port->dports))
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 514 rc = -EINVAL;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 515
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 516 for (i = 0; rc == 0 && target_map && i < cxld->nr_targets; i++) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 517 struct cxl_dport *dport = find_dport(port, target_map[i]);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 518
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 519 if (!dport) {
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 520 rc = -ENXIO;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 521 break;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 522 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 523 dev_dbg(host, "%s: target: %d\n", dev_name(dport->dport), i);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 524 cxld->target[i] = dport;
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 525 }
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 526 device_unlock(&port->dev);
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 527 if (rc)
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 528 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 529
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 530 dev = &cxld->dev;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 531 rc = dev_set_name(dev, "decoder%d.%d", port->id, cxld->id);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 532 if (rc)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 533 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 534
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 535 rc = device_add(dev);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 536 if (rc)
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 537 goto err;
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 538
> b7ca54b6255144 drivers/cxl/core/bus.c Dan Williams 2021-08-24 539 return devm_add_action_or_reset(host, unregister_cxl_dev, dev);
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 540 err:
> 40ba17afdfabb0 drivers/cxl/core.c Dan Williams 2021-06-09 @541 put_device(dev);
>
> Should be:
>
> put_device(&cxld->dev);
>
> But it feels like a layering violation to drop a reference that was
> aquired by the caller.
This code hit linux-next yesterday so I reviewed it in context. The
put_device() should just be removed. It leads to a use after free.
regards,
dan carpenter
next prev parent reply other threads:[~2021-08-26 12:50 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-24 23:47 [cxl-cxl:pending 39/40] drivers/cxl/core/bus.c:501 devm_cxl_add_decoder() warn: variable dereferenced before check 'cxld' (see line 497) kernel test robot
2021-08-25 7:12 ` Dan Carpenter
2021-08-25 7:12 ` Dan Carpenter
2021-08-26 12:50 ` Dan Carpenter [this message]
2021-08-26 12:50 ` Dan Carpenter
2021-08-26 12:50 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210826125020.GA7722@kadam \
--to=dan.carpenter@oracle.com \
--cc=kbuild@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.