Coverity: ucsi_check_cable(): Null pointer dereferences

DISCUSSION coverity-bot 2 participants 3 messages Mar 7, 2024

Subsystems (2)

USB Linux-next tree

lore

Share & tools

lore

coverity-bot

author

2024-03-07 19:34 UTC #0 lore raw

Hello!

This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20240307 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan

You're getting this email because you were associated with the identified lines of code (noted below) that were touched by commits:

Tue Mar 5 13:11:08 2024 +0000
f896d5e8726c ("usb: typec: ucsi: Register SOP/SOP' Discover Identity Responses")

Coverity reported the following:

*** CID 1584245: Null pointer dereferences (FORWARD_NULL) drivers/usb/typec/ucsi/ucsi.c:1136 in ucsi_check_cable()
1130 }
1131

1132     	ret = ucsi_register_cable(con);
1133     	if (ret < 0)
1134     		return ret;
1135
vvv     CID 1584245:  Null pointer dereferences  (FORWARD_NULL)
vvv     Passing "con" to "ucsi_get_cable_identity", which dereferences null "con->cable".
1136     	ret = ucsi_get_cable_identity(con);
1137     	if (ret < 0)
1138     		return ret;

1139
1140 ret = ucsi_register_plug(con);
1141 if (ret < 0)

If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):

Reported-by coverity-bot <[email protected]>

Addresses-Coverity-ID: 1584245 ("Null pointer dereferences")

Fixes f896d5e8726c ("usb: typec: ucsi: Register SOP/SOP' Discover Identity Responses")

Thanks for your attention!

Signature · 1 line

Coverity-bot

Christian A. Ehrhardt

2024-03-07 20:16 UTC #1 lore raw

Hi,

Quoted 27 lines · coverity-bot

Hello!
This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20240307 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan
You're getting this email because you were associated with the identified lines of code (noted below) that were touched by commits:
Tue Mar 5 13:11:08 2024 +0000
f896d5e8726c ("usb: typec: ucsi: Register SOP/SOP' Discover Identity Responses")
Coverity reported the following:
*** CID 1584245: Null pointer dereferences (FORWARD_NULL) drivers/usb/typec/ucsi/ucsi.c:1136 in ucsi_check_cable()
1130 }
1131
1132     	ret = ucsi_register_cable(con);
1133     	if (ret < 0)
1134     		return ret;
1135
vvv     CID 1584245:  Null pointer dereferences  (FORWARD_NULL)
vvv     Passing "con" to "ucsi_get_cable_identity", which dereferences null "con->cable".
1136     	ret = ucsi_get_cable_identity(con);
1137     	if (ret < 0)
1138     		return ret;

Quoted 8 lines

1139
1140 ret = ucsi_register_plug(con);
1141 if (ret < 0)
If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):

This looks like a false positive to me. The code looks like this:

	if (con->cable)
		return 0;
	[ ... ]
	ret = ucsi_register_cable(con)
	if (ret < 0)
		return ret;
	ret = ucsi_get_cable_identity(con);
	[ ... ]

From the con->cable check coverity concludes that con->cable is initially NULL. Later ucsi_register_cable() initializes con->cable if successful. Coverity seems to miss this and still thinks that con->cable is NULL. Then converity correctly notices that ucsi_get_cable_identity() dereferences con->cable and complains.

regards Christian

Kees Cook

author

2024-03-07 20:22 UTC #2 lore raw

Quoted 51 lines · Christian A. Ehrhardt

Hi,
Quoted 27 lines · coverity-bot
Hello!
This is an experimental semi-automated report about issues detected by Coverity from a scan of next-20240307 as part of the linux-next scan project:
https://scan.coverity.com/projects/linux-next-weekly-scan
You're getting this email because you were associated with the identified lines of code (noted below) that were touched by commits:
Tue Mar 5 13:11:08 2024 +0000
f896d5e8726c ("usb: typec: ucsi: Register SOP/SOP' Discover Identity Responses")
Coverity reported the following:
*** CID 1584245: Null pointer dereferences (FORWARD_NULL) drivers/usb/typec/ucsi/ucsi.c:1136 in ucsi_check_cable()
1130 }
1131
1132     	ret = ucsi_register_cable(con);
1133     	if (ret < 0)
1134     		return ret;
1135
vvv     CID 1584245:  Null pointer dereferences  (FORWARD_NULL)
vvv     Passing "con" to "ucsi_get_cable_identity", which dereferences null "con->cable".
1136     	ret = ucsi_get_cable_identity(con);
1137     	if (ret < 0)
1138     		return ret;
Quoted 8 lines
1139
1140 ret = ucsi_register_plug(con);
1141 if (ret < 0)
If this is a false positive, please let us know so we can mark it as such, or teach the Coverity rules to be smarter. If not, please make sure fixes get into linux-next. :) For patches fixing this, please include these lines (but double-check the "Fixes" first):
This looks like a false positive to me. The code looks like this:
	if (con->cable)
		return 0;
	[ ... ]
	ret = ucsi_register_cable(con)
	if (ret < 0)
		return ret;
	ret = ucsi_get_cable_identity(con);
	[ ... ]

Quoted 5 lines

From the con->cable check coverity concludes that con->cable is initially NULL. Later ucsi_register_cable() initializes con->cable if successful. Coverity seems to miss this and still thinks that con->cable is NULL. Then converity correctly notices that ucsi_get_cable_identity() dereferences con->cable and complains.

Ah-ha! Yes, the ucsi_register_cable() check seems to have been missed. I think it's confused by:

        cable = typec_register_cable(con->port, &desc);
        if (IS_ERR(cable)) {

This isn't IS_ERR_OR_NULL, so it thinks cable might still be NULL, but there's no path through typec_register_cable() where that can be true.

Thanks for taking a look!

-Kees

Signature · 1 line

Kees Cook