block: Check blkg_to_lat return value to avoid NULL dereference

[PATCH] block: Check blkg_to_lat return value to avoid NULL dereference

Posted by Wentao Liang 10 months, 1 week ago

The function blkg_to_lat() may return NULL if the blkg is not associated
with an iolatency group. In iolatency_set_min_lat_nsec() and
iolatency_pd_init(), the return values are not checked, leading to
potential NULL pointer dereferences.

This patch adds checks for the return values of blkg_to_lat and let it
returns early if it is NULL, preventing the NULL pointer dereference.

Fixes: d70675121546 ("block: introduce blk-iolatency io controller")
Cc: stable@vger.kernel.org # 4.19+
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 block/blk-iolatency.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index ebb522788d97..398f0a1747c4 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -787,6 +787,8 @@ static int blk_iolatency_init(struct gendisk *disk)
 static void iolatency_set_min_lat_nsec(struct blkcg_gq *blkg, u64 val)
 {
 	struct iolatency_grp *iolat = blkg_to_lat(blkg);
+	if (!iolat)
+		return;
 	struct blk_iolatency *blkiolat = iolat->blkiolat;
 	u64 oldval = iolat->min_lat_nsec;
 
@@ -1013,6 +1015,8 @@ static void iolatency_pd_init(struct blkg_policy_data *pd)
 	 */
 	if (blkg->parent && blkg_to_pd(blkg->parent, &blkcg_policy_iolatency)) {
 		struct iolatency_grp *parent = blkg_to_lat(blkg->parent);
+		if (!parent)
+			return;
 		atomic_set(&iolat->scale_cookie,
 			   atomic_read(&parent->child_lat.scale_cookie));
 	} else {
-- 
2.42.0.windows.2

Re: [PATCH] block: Check blkg_to_lat return value to avoid NULL dereference

Posted by Yu Kuai 10 months, 1 week ago

Hi,

在 2025/02/12 16:32, Wentao Liang 写道:
> The function blkg_to_lat() may return NULL if the blkg is not associated
> with an iolatency group. In iolatency_set_min_lat_nsec() and
> iolatency_pd_init(), the return values are not checked, leading to
> potential NULL pointer dereferences.
> 
> This patch adds checks for the return values of blkg_to_lat and let it
> returns early if it is NULL, preventing the NULL pointer dereference.
> 
> Fixes: d70675121546 ("block: introduce blk-iolatency io controller")
> Cc: stable@vger.kernel.org # 4.19+
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   block/blk-iolatency.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
> index ebb522788d97..398f0a1747c4 100644
> --- a/block/blk-iolatency.c
> +++ b/block/blk-iolatency.c
> @@ -787,6 +787,8 @@ static int blk_iolatency_init(struct gendisk *disk)
>   static void iolatency_set_min_lat_nsec(struct blkcg_gq *blkg, u64 val)
>   {
>   	struct iolatency_grp *iolat = blkg_to_lat(blkg);
> +	if (!iolat)
> +		return;
>   	struct blk_iolatency *blkiolat = iolat->blkiolat;
>   	u64 oldval = iolat->min_lat_nsec;

This is not needed, this is called from iolatency_set_limit() or
iolatency_pd_offline() where the policy data can't be NULL.
>   
> @@ -1013,6 +1015,8 @@ static void iolatency_pd_init(struct blkg_policy_data *pd)
>   	 */
>   	if (blkg->parent && blkg_to_pd(blkg->parent, &blkcg_policy_iolatency)) {
>   		struct iolatency_grp *parent = blkg_to_lat(blkg->parent);
> +		if (!parent)
> +			return;

blkg_to_pd() already checked, how can this be NULL?

Thanks,
Kuai
>   		atomic_set(&iolat->scale_cookie,
>   			   atomic_read(&parent->child_lat.scale_cookie));
>   	} else {
>

Re: [PATCH] block: Check blkg_to_lat return value to avoid NULL dereference

Posted by Johannes Thumshirn 10 months, 1 week ago

On 12.02.25 09:32, Wentao Liang wrote:
> The function blkg_to_lat() may return NULL if the blkg is not associated
> with an iolatency group. In iolatency_set_min_lat_nsec() and
> iolatency_pd_init(), the return values are not checked, leading to
> potential NULL pointer dereferences.
> 
> This patch adds checks for the return values of blkg_to_lat and let it
> returns early if it is NULL, preventing the NULL pointer dereference.
> 
> Fixes: d70675121546 ("block: introduce blk-iolatency io controller")
> Cc: stable@vger.kernel.org # 4.19+
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   block/blk-iolatency.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
> index ebb522788d97..398f0a1747c4 100644
> --- a/block/blk-iolatency.c
> +++ b/block/blk-iolatency.c
> @@ -787,6 +787,8 @@ static int blk_iolatency_init(struct gendisk *disk)
>   static void iolatency_set_min_lat_nsec(struct blkcg_gq *blkg, u64 val)
>   {
>   	struct iolatency_grp *iolat = blkg_to_lat(blkg);
> +	if (!iolat)
> +		return;
>   	struct blk_iolatency *blkiolat = iolat->blkiolat;
>   	u64 oldval = iolat->min_lat_nsec;
>   

Uh that looks horrible. I haven't checked the surrounding code but 
please please at least make it

static void iolatency_set_min_lat_nsec(struct blkcg_gq *blkg, u64 val)
{
	struct iolatency_grp *iolat = blkg_to_lat(blkg);
	struct blk_iolatency *blkiolat;
	u64 oldval;

	if (!iolat)
		return;

	blkiolat = iolat->blkiolat;
	oldval = iolat->min_lat_nsec;



> @@ -1013,6 +1015,8 @@ static void iolatency_pd_init(struct blkg_policy_data *pd)
>   	 */
>   	if (blkg->parent && blkg_to_pd(blkg->parent, &blkcg_policy_iolatency)) {
>   		struct iolatency_grp *parent = blkg_to_lat(blkg->parent);
> +		if (!parent)
> +			return;
>   		atomic_set(&iolat->scale_cookie,
>   			   atomic_read(&parent->child_lat.scale_cookie));
>   	} else {

Re: [PATCH] block: Check blkg_to_lat() return value to avoid NULL dereference

Posted by Markus Elfring 10 months, 1 week ago

…
> This patch adds checks for the return values of blkg_to_lat and let it

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.14-rc2#n94


> returns early if it is NULL, preventing the NULL pointer dereference.

  return?


Regards,
Markus