* [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver
@ 2012-04-09 4:16 Olof Johansson
2012-04-09 8:46 ` Jingoo Han
2012-04-09 9:25 ` Jingoo Han
0 siblings, 2 replies; 5+ messages in thread
From: Olof Johansson @ 2012-04-09 4:16 UTC (permalink / raw)
To: Jingoo Han
Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel, Olof Johansson
I'm not 100% sure if the fix for 'adjust_request' is correct, since
it's uncertain what the original intent was. But it's so clearly an
uninitialized pointer dereference that my resolution seems to make sense.
drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in this function [-Wuninitialized]
drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here
drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function [-Wuninitialized]
drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
Signed-off-by: Olof Johansson <olof@lixom.net>
---
drivers/video/exynos/exynos_dp_core.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
index 2a4481c..8973e18 100644
--- a/drivers/video/exynos/exynos_dp_core.c
+++ b/drivers/video/exynos/exynos_dp_core.c
@@ -392,24 +392,19 @@ static unsigned int exynos_dp_get_lane_link_training(
struct exynos_dp_device *dp,
int lane)
{
- u32 reg;
-
switch (lane) {
case 0:
- reg = exynos_dp_get_lane0_link_training(dp);
- break;
+ return exynos_dp_get_lane0_link_training(dp);
case 1:
- reg = exynos_dp_get_lane1_link_training(dp);
- break;
+ return exynos_dp_get_lane1_link_training(dp);
case 2:
- reg = exynos_dp_get_lane2_link_training(dp);
- break;
+ return exynos_dp_get_lane2_link_training(dp);
case 3:
- reg = exynos_dp_get_lane3_link_training(dp);
- break;
+ return exynos_dp_get_lane3_link_training(dp);
}
- return reg;
+ WARN_ON(1);
+ return 0;
}
static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
@@ -489,13 +484,13 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
6, link_status);
lane_count = dp->link_train.lane_count;
+ adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
+ - DPCD_ADDR_LANE0_1_STATUS);
+
if (exynos_dp_clock_recovery_ok(link_status, lane_count) == 0) {
/* set training pattern 2 for EQ */
exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
- adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
- - DPCD_ADDR_LANE0_1_STATUS);
-
exynos_dp_get_adjust_train(dp, adjust_request);
buf[0] = DPCD_SCRAMBLING_DISABLED |
--
1.7.9.2.359.gebfc2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver
2012-04-09 4:16 [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver Olof Johansson
@ 2012-04-09 8:46 ` Jingoo Han
2012-04-09 14:05 ` Olof Johansson
2012-04-09 9:25 ` Jingoo Han
1 sibling, 1 reply; 5+ messages in thread
From: Jingoo Han @ 2012-04-09 8:46 UTC (permalink / raw)
To: 'Olof Johansson'
Cc: 'Florian Tobias Schandinat', linux-fbdev, linux-kernel
> -----Original Message-----
> From: Olof Johansson [mailto:olof@lixom.net]
> Sent: Monday, April 09, 2012 1:16 PM
>
> I'm not 100% sure if the fix for 'adjust_request' is correct, since
> it's uncertain what the original intent was. But it's so clearly an
> uninitialized pointer dereference that my resolution seems to make sense.
>
> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
> drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in
> this function [-Wuninitialized]
> drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here
> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function
> [-Wuninitialized]
> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
> drivers/video/exynos/exynos_dp_core.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 2a4481c..8973e18 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -392,24 +392,19 @@ static unsigned int exynos_dp_get_lane_link_training(
> struct exynos_dp_device *dp,
> int lane)
> {
> - u32 reg;
> -
> switch (lane) {
> case 0:
> - reg = exynos_dp_get_lane0_link_training(dp);
> - break;
> + return exynos_dp_get_lane0_link_training(dp);
> case 1:
> - reg = exynos_dp_get_lane1_link_training(dp);
> - break;
> + return exynos_dp_get_lane1_link_training(dp);
> case 2:
> - reg = exynos_dp_get_lane2_link_training(dp);
> - break;
> + return exynos_dp_get_lane2_link_training(dp);
> case 3:
> - reg = exynos_dp_get_lane3_link_training(dp);
> - break;
> + return exynos_dp_get_lane3_link_training(dp);
> }
I don't like multi return.
>
> - return reg;
> + WARN_ON(1);
> + return 0;
> }
>
> static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
> @@ -489,13 +484,13 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> 6, link_status);
> lane_count = dp->link_train.lane_count;
>
> + adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
> + - DPCD_ADDR_LANE0_1_STATUS);
> +
It makes the problem. adjust_request will be different.
OK, I understand what you want to do.
I will send the version 2 patch which is simpler.
Thank you for sending the patch.
> if (exynos_dp_clock_recovery_ok(link_status, lane_count) == 0) {
> /* set training pattern 2 for EQ */
> exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
>
> - adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
> - - DPCD_ADDR_LANE0_1_STATUS);
> -
> exynos_dp_get_adjust_train(dp, adjust_request);
>
> buf[0] = DPCD_SCRAMBLING_DISABLED |
> --
> 1.7.9.2.359.gebfc2
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver
2012-04-09 4:16 [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver Olof Johansson
2012-04-09 8:46 ` Jingoo Han
@ 2012-04-09 9:25 ` Jingoo Han
2012-04-09 14:07 ` Olof Johansson
1 sibling, 1 reply; 5+ messages in thread
From: Jingoo Han @ 2012-04-09 9:25 UTC (permalink / raw)
To: 'Olof Johansson'
Cc: 'Florian Tobias Schandinat', linux-fbdev, linux-kernel
> -----Original Message-----
> From: Olof Johansson [mailto:olof@lixom.net]
> Sent: Monday, April 09, 2012 1:16 PM
>
> I'm not 100% sure if the fix for 'adjust_request' is correct, since
> it's uncertain what the original intent was. But it's so clearly an
> uninitialized pointer dereference that my resolution seems to make sense.
>
> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
> drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in
> this function [-Wuninitialized]
> drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here
> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function
> [-Wuninitialized]
> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
My compiler cannot detect this warning.
What is your compiler version?
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
> ---
> drivers/video/exynos/exynos_dp_core.c | 23 +++++++++--------------
> 1 file changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
> index 2a4481c..8973e18 100644
> --- a/drivers/video/exynos/exynos_dp_core.c
> +++ b/drivers/video/exynos/exynos_dp_core.c
> @@ -392,24 +392,19 @@ static unsigned int exynos_dp_get_lane_link_training(
> struct exynos_dp_device *dp,
> int lane)
> {
> - u32 reg;
> -
> switch (lane) {
> case 0:
> - reg = exynos_dp_get_lane0_link_training(dp);
> - break;
> + return exynos_dp_get_lane0_link_training(dp);
> case 1:
> - reg = exynos_dp_get_lane1_link_training(dp);
> - break;
> + return exynos_dp_get_lane1_link_training(dp);
> case 2:
> - reg = exynos_dp_get_lane2_link_training(dp);
> - break;
> + return exynos_dp_get_lane2_link_training(dp);
> case 3:
> - reg = exynos_dp_get_lane3_link_training(dp);
> - break;
> + return exynos_dp_get_lane3_link_training(dp);
> }
>
> - return reg;
> + WARN_ON(1);
> + return 0;
> }
>
> static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
> @@ -489,13 +484,13 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
> 6, link_status);
> lane_count = dp->link_train.lane_count;
>
> + adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
> + - DPCD_ADDR_LANE0_1_STATUS);
> +
> if (exynos_dp_clock_recovery_ok(link_status, lane_count) == 0) {
> /* set training pattern 2 for EQ */
> exynos_dp_set_training_pattern(dp, TRAINING_PTN2);
>
> - adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
> - - DPCD_ADDR_LANE0_1_STATUS);
> -
> exynos_dp_get_adjust_train(dp, adjust_request);
>
> buf[0] = DPCD_SCRAMBLING_DISABLED |
> --
> 1.7.9.2.359.gebfc2
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver
2012-04-09 8:46 ` Jingoo Han
@ 2012-04-09 14:05 ` Olof Johansson
0 siblings, 0 replies; 5+ messages in thread
From: Olof Johansson @ 2012-04-09 14:05 UTC (permalink / raw)
To: Jingoo Han; +Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel
On Mon, Apr 9, 2012 at 1:46 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>> -----Original Message-----
>> From: Olof Johansson [mailto:olof@lixom.net]
>> Sent: Monday, April 09, 2012 1:16 PM
>>
>> I'm not 100% sure if the fix for 'adjust_request' is correct, since
>> it's uncertain what the original intent was. But it's so clearly an
>> uninitialized pointer dereference that my resolution seems to make sense.
>>
>> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
>> drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in
>> this function [-Wuninitialized]
>> drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here
>> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function
>> [-Wuninitialized]
>> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
>>
>> Signed-off-by: Olof Johansson <olof@lixom.net>
>> ---
>> drivers/video/exynos/exynos_dp_core.c | 23 +++++++++--------------
>> 1 file changed, 9 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/video/exynos/exynos_dp_core.c b/drivers/video/exynos/exynos_dp_core.c
>> index 2a4481c..8973e18 100644
>> --- a/drivers/video/exynos/exynos_dp_core.c
>> +++ b/drivers/video/exynos/exynos_dp_core.c
>> @@ -392,24 +392,19 @@ static unsigned int exynos_dp_get_lane_link_training(
>> struct exynos_dp_device *dp,
>> int lane)
>> {
>> - u32 reg;
>> -
>> switch (lane) {
>> case 0:
>> - reg = exynos_dp_get_lane0_link_training(dp);
>> - break;
>> + return exynos_dp_get_lane0_link_training(dp);
>> case 1:
>> - reg = exynos_dp_get_lane1_link_training(dp);
>> - break;
>> + return exynos_dp_get_lane1_link_training(dp);
>> case 2:
>> - reg = exynos_dp_get_lane2_link_training(dp);
>> - break;
>> + return exynos_dp_get_lane2_link_training(dp);
>> case 3:
>> - reg = exynos_dp_get_lane3_link_training(dp);
>> - break;
>> + return exynos_dp_get_lane3_link_training(dp);
>> }
>
> I don't like multi return.
In a small helper function like this there's nothing wrong with it.
Larger functions? Sure.
Adding a default in the switch didn't seem like an improvement to me.
But I'll leave it up to you.
>>
>> - return reg;
>> + WARN_ON(1);
>> + return 0;
>> }
>>
>> static void exynos_dp_reduce_link_rate(struct exynos_dp_device *dp)
>> @@ -489,13 +484,13 @@ static int exynos_dp_process_clock_recovery(struct exynos_dp_device *dp)
>> 6, link_status);
>> lane_count = dp->link_train.lane_count;
>>
>> + adjust_request = link_status + (DPCD_ADDR_ADJUST_REQUEST_LANE0_1
>> + - DPCD_ADDR_LANE0_1_STATUS);
>> +
>
> It makes the problem. adjust_request will be different.
>
> OK, I understand what you want to do.
> I will send the version 2 patch which is simpler.
>
> Thank you for sending the patch.
Ok, thanks. As I said, I'm not sure what your intent with the second
(else) code path was.
-Olof
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver
2012-04-09 9:25 ` Jingoo Han
@ 2012-04-09 14:07 ` Olof Johansson
0 siblings, 0 replies; 5+ messages in thread
From: Olof Johansson @ 2012-04-09 14:07 UTC (permalink / raw)
To: Jingoo Han; +Cc: Florian Tobias Schandinat, linux-fbdev, linux-kernel
On Mon, Apr 9, 2012 at 2:25 AM, Jingoo Han <jg1.han@samsung.com> wrote:
>> -----Original Message-----
>> From: Olof Johansson [mailto:olof@lixom.net]
>> Sent: Monday, April 09, 2012 1:16 PM
>>
>> I'm not 100% sure if the fix for 'adjust_request' is correct, since
>> it's uncertain what the original intent was. But it's so clearly an
>> uninitialized pointer dereference that my resolution seems to make sense.
>>
>> drivers/video/exynos/exynos_dp_core.c: In function 'exynos_dp_set_link_train':
>> drivers/video/exynos/exynos_dp_core.c:521:21: warning: 'adjust_request' may be used uninitialized in
>> this function [-Wuninitialized]
>> drivers/video/exynos/exynos_dp_core.c:481:6: note: 'adjust_request' was declared here
>> drivers/video/exynos/exynos_dp_core.c:529:18: warning: 'reg' may be used uninitialized in this function
>> [-Wuninitialized]
>> drivers/video/exynos/exynos_dp_core.c:395:6: note: 'reg' was declared here
>
> My compiler cannot detect this warning.
> What is your compiler version?
gcc version 4.6.2 (GCC)
-Olof
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-04-09 14:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-09 4:16 [PATCH] video: exynos: fix build warning and bad pointer deref in dp driver Olof Johansson
2012-04-09 8:46 ` Jingoo Han
2012-04-09 14:05 ` Olof Johansson
2012-04-09 9:25 ` Jingoo Han
2012-04-09 14:07 ` Olof Johansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).