diff mbox

[2/2] drm/dp: Don't trust drm_dp_downstream_id()

Message ID 20170720174532.23377-2-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson July 20, 2017, 5:45 p.m. UTC
Before we interpret drm_dp_downstream_id() as a string, make sure it is
NULL terminated, even when drm_dp_downtsream_id() fails.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101660
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/drm_dp_helper.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Jani Nikula July 21, 2017, 7:22 a.m. UTC | #1
On Thu, 20 Jul 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Before we interpret drm_dp_downstream_id() as a string, make sure it is
> NULL terminated, even when drm_dp_downtsream_id() fails.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101660
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index 1d9e30f0dbf8..08af8d6b844b 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>  				 DP_DETAILED_CAP_INFO_AVAILABLE;
>  	int clk;
>  	int bpc;
> -	char id[6];
> +	char id[7];
>  	int len;
>  	uint8_t rev[2];
>  	int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> @@ -583,6 +583,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>  		seq_puts(m, "\t\tType: N/A\n");
>  	}
>  
> +	memset(id, 0, sizeof(id));
>  	drm_dp_downstream_id(aux, id);
>  	seq_printf(m, "\t\tID: %s\n", id);

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

although I'd probably have gone for checking the return value of
drm_dp_downstream_id() and using %*pE format with strnlen for the
length.
Jani Nikula July 21, 2017, 2:51 p.m. UTC | #2
On Fri, 21 Jul 2017, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 20 Jul 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> Before we interpret drm_dp_downstream_id() as a string, make sure it is
>> NULL terminated, even when drm_dp_downtsream_id() fails.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101660
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/drm_dp_helper.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> index 1d9e30f0dbf8..08af8d6b844b 100644
>> --- a/drivers/gpu/drm/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/drm_dp_helper.c
>> @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>>  				 DP_DETAILED_CAP_INFO_AVAILABLE;
>>  	int clk;
>>  	int bpc;
>> -	char id[6];
>> +	char id[7];
>>  	int len;
>>  	uint8_t rev[2];
>>  	int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
>> @@ -583,6 +583,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>>  		seq_puts(m, "\t\tType: N/A\n");
>>  	}
>>  
>> +	memset(id, 0, sizeof(id));
>>  	drm_dp_downstream_id(aux, id);
>>  	seq_printf(m, "\t\tID: %s\n", id);
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
> although I'd probably have gone for checking the return value of
> drm_dp_downstream_id() and using %*pE format with strnlen for the
> length.

And pushed both to drm-misc-fixes, thanks for the patches.

BR,
Jani.
Chris Wilson July 21, 2017, 3:01 p.m. UTC | #3
Quoting Jani Nikula (2017-07-21 08:22:27)
> On Thu, 20 Jul 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Before we interpret drm_dp_downstream_id() as a string, make sure it is
> > NULL terminated, even when drm_dp_downtsream_id() fails.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101660
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/drm_dp_helper.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> > index 1d9e30f0dbf8..08af8d6b844b 100644
> > --- a/drivers/gpu/drm/drm_dp_helper.c
> > +++ b/drivers/gpu/drm/drm_dp_helper.c
> > @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
> >                                DP_DETAILED_CAP_INFO_AVAILABLE;
> >       int clk;
> >       int bpc;
> > -     char id[6];
> > +     char id[7];
> >       int len;
> >       uint8_t rev[2];
> >       int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
> > @@ -583,6 +583,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
> >               seq_puts(m, "        Type: N/A\n");
> >       }
> >  
> > +     memset(id, 0, sizeof(id));
> >       drm_dp_downstream_id(aux, id);
> >       seq_printf(m, "        ID: %s\n", id);
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> although I'd probably have gone for checking the return value of
> drm_dp_downstream_id() and using %*pE format with strnlen for the
> length.

I'm also wondering if we should treat it as a string? Print hexadecimal
+ ascii? There's probably a printf specifier for that as well.
-Chris
Jani Nikula July 21, 2017, 3:14 p.m. UTC | #4
On Fri, 21 Jul 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Quoting Jani Nikula (2017-07-21 08:22:27)
>> On Thu, 20 Jul 2017, Chris Wilson <chris@chris-wilson.co.uk> wrote:
>> > Before we interpret drm_dp_downstream_id() as a string, make sure it is
>> > NULL terminated, even when drm_dp_downtsream_id() fails.
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101660
>> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > ---
>> >  drivers/gpu/drm/drm_dp_helper.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
>> > index 1d9e30f0dbf8..08af8d6b844b 100644
>> > --- a/drivers/gpu/drm/drm_dp_helper.c
>> > +++ b/drivers/gpu/drm/drm_dp_helper.c
>> > @@ -544,7 +544,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>> >                                DP_DETAILED_CAP_INFO_AVAILABLE;
>> >       int clk;
>> >       int bpc;
>> > -     char id[6];
>> > +     char id[7];
>> >       int len;
>> >       uint8_t rev[2];
>> >       int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
>> > @@ -583,6 +583,7 @@ void drm_dp_downstream_debug(struct seq_file *m,
>> >               seq_puts(m, "        Type: N/A\n");
>> >       }
>> >  
>> > +     memset(id, 0, sizeof(id));
>> >       drm_dp_downstream_id(aux, id);
>> >       seq_printf(m, "        ID: %s\n", id);
>> 
>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>> 
>> although I'd probably have gone for checking the return value of
>> drm_dp_downstream_id() and using %*pE format with strnlen for the
>> length.
>
> I'm also wondering if we should treat it as a string? Print hexadecimal
> + ascii? There's probably a printf specifier for that as well.

Missed your mail, pushed already. The rest is for another patch, another
day.

BR,
Jani.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index 1d9e30f0dbf8..08af8d6b844b 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -544,7 +544,7 @@  void drm_dp_downstream_debug(struct seq_file *m,
 				 DP_DETAILED_CAP_INFO_AVAILABLE;
 	int clk;
 	int bpc;
-	char id[6];
+	char id[7];
 	int len;
 	uint8_t rev[2];
 	int type = port_cap[0] & DP_DS_PORT_TYPE_MASK;
@@ -583,6 +583,7 @@  void drm_dp_downstream_debug(struct seq_file *m,
 		seq_puts(m, "\t\tType: N/A\n");
 	}
 
+	memset(id, 0, sizeof(id));
 	drm_dp_downstream_id(aux, id);
 	seq_printf(m, "\t\tID: %s\n", id);