From: Dan Carpenter <dan.carpenter@linaro.org>
To: Li Zhijian <lizhijian@fujitsu.com>
Cc: dave@stgolabs.net, jonathan.cameron@huawei.com,
dave.jiang@intel.com, alison.schofield@intel.com,
vishal.l.verma@intel.com, ira.weiny@intel.com,
dan.j.williams@intel.com, linux-cxl@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] cxl/region: Fix potential invalid pointer dereference
Date: Mon, 29 Apr 2024 13:10:52 +0300 [thread overview]
Message-ID: <18a295a2-b734-45dd-9c95-660a9e1f58e4@moroto.mountain> (raw)
In-Reply-To: <20240429013154.368118-1-lizhijian@fujitsu.com>
On Mon, Apr 29, 2024 at 09:31:53AM +0800, Li Zhijian wrote:
> construct_region() could return a PTR_ERR() which cannot be derefernced.
> Moving the dereference behind the error checking to make sure the
> pointer is valid.
>
No, this patch is unnecessary.
drivers/cxl/core/region.c
3080 /*
3081 * Ensure that if multiple threads race to construct_region() for @hpa
3082 * one does the construction and the others add to that.
3083 */
3084 mutex_lock(&cxlrd->range_lock);
3085 region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
3086 match_region_by_range);
3087 if (!region_dev) {
3088 cxlr = construct_region(cxlrd, cxled);
3089 region_dev = &cxlr->dev;
^^^^^^^^^^^
This is not a dereference, it's just pointer math. In in this case it's
the same as saying:
region_dev = (void *)cxlr;
3090 } else
3091 cxlr = to_cxl_region(region_dev);
3092 mutex_unlock(&cxlrd->range_lock);
3093
3094 rc = PTR_ERR_OR_ZERO(cxlr);
^^^^^^^^^^^^^^^^^^^^^^^^^^^
This check means that if cxlr is an error pointer then we will clean up
and return an error.
regards,
dan carpenter
3095 if (rc)
3096 goto out;
3097
3098 attach_target(cxlr, cxled, -1, TASK_UNINTERRUPTIBLE);
3099
3100 down_read(&cxl_region_rwsem);
3101 p = &cxlr->params;
3102 attach = p->state == CXL_CONFIG_COMMIT;
3103 up_read(&cxl_region_rwsem);
3104
3105 if (attach) {
3106 /*
3107 * If device_attach() fails the range may still be active via
3108 * the platform-firmware memory map, otherwise the driver for
3109 * regions is local to this file, so driver matching can't fail.
3110 */
3111 if (device_attach(&cxlr->dev) < 0)
3112 dev_err(&cxlr->dev, "failed to enable, range: %pr\n",
3113 p->res);
3114 }
3115
3116 put_device(region_dev);
3117 out:
3118 put_device(cxlrd_dev);
3119 return rc;
3120 }
next prev parent reply other threads:[~2024-04-29 10:10 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 1:31 [PATCH 1/2] cxl/region: Fix potential invalid pointer dereference Li Zhijian
2024-04-29 1:31 ` [PATCH 2/2] cxl/region: Fix missing put_device(region_dev) Li Zhijian
2024-04-29 1:51 ` Zhijian Li (Fujitsu)
2024-04-29 8:00 ` Markus Elfring
2024-04-29 8:26 ` Zhijian Li (Fujitsu)
2024-04-29 10:00 ` Dan Carpenter
2024-04-29 10:11 ` Zhijian Li (Fujitsu)
2024-04-29 8:35 ` Zhijian Li (Fujitsu)
2024-04-29 10:17 ` Dan Carpenter
2024-04-29 10:26 ` Zhijian Li (Fujitsu)
2024-04-29 10:32 ` Dan Carpenter
2024-04-29 16:14 ` Ira Weiny
2024-04-29 7:50 ` [PATCH 1/2] cxl/region: Fix potential invalid pointer dereference Markus Elfring
2024-04-29 8:43 ` Zhijian Li (Fujitsu)
2024-04-29 8:55 ` [1/2] " Markus Elfring
2024-04-29 10:10 ` Dan Carpenter [this message]
2024-04-29 10:25 ` [PATCH 1/2] " Zhijian Li (Fujitsu)
2024-04-29 10:30 ` Dan Carpenter
2024-04-29 16:17 ` Ira Weiny
2024-04-29 16:05 ` Ira Weiny
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=18a295a2-b734-45dd-9c95-660a9e1f58e4@moroto.mountain \
--to=dan.carpenter@linaro.org \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dave@stgolabs.net \
--cc=ira.weiny@intel.com \
--cc=jonathan.cameron@huawei.com \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=lizhijian@fujitsu.com \
--cc=vishal.l.verma@intel.com \
/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.