RE: [PATCH] drm/i915/display: Initalizalize capability variables
> -Original Message-
> From: Borah, Chaitanya Kumar
> Sent: Wednesday, March 27, 2024 9:32 AM
> To: Kandpal, Suraj ; intel-gfx@lists.freedesktop.org
> Subject: RE: [PATCH] drm/i915/display: Initalizalize capability variables
>
>
>
> > -Original Message-
> > From: Kandpal, Suraj
> > Sent: Wednesday, March 27, 2024 9:06 AM
> > To: Borah, Chaitanya Kumar ; intel-
> > g...@lists.freedesktop.org
> > Subject: RE: [PATCH] drm/i915/display: Initalizalize capability
> > variables
> >
> > > Hello Suraj,
> > >
> > > > -Original Message-
> > > > From: Kandpal, Suraj
> > > > Sent: Tuesday, March 26, 2024 10:45 AM
> > > > To: intel-gfx@lists.freedesktop.org
> > > > Cc: Borah, Chaitanya Kumar ;
> > > > Kandpal, Suraj
> > > > Subject: [PATCH] drm/i915/display: Initalizalize capability
> > > > variables
> > >
> > > Typo: Initialize
> > >
> >
> > Sure will fix
> >
> > > > Initialize HDCP capability variables to false to avoid UBSAN
> > > > warning in boolean value.
> > > >
> > >
> > > I can see a case where hdcp_cap remains unassigned in case
> > > intel_dp_hdcp_get_remote_capability() returns without assigning it a
> value.
> > >
> > > Is that intended/expected?
> > >
> >
> > That is not intentional but this patch makes sure hdcp_capable will be
> > false when this function gets called.
> >
>
> This patch makes sense for sure but what I am getting at is if UBSAN is
> revealing a bug in our code.
>
> intel_dp_hdcp_get_remote_capability() returns if
> _intel_dp_hdcp2_get_capability() fails. In that case we never check for hdcp
> 1.4 capability.
> If a remote sink can be hdcp 1.4 capable but not hdcp 2 capable, this logic is
> problematic.
True let me add a patch that fixes that too.
Regards,
Suraj Kandpal
>
> Regards
>
> Chaitanya
>
> > Regards,
> > Suraj Kandpal
> >
> > > Regards
> > >
> > > Chaitanya
> > >
> > > > Signed-off-by: Suraj Kandpal
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > index b99c024b0934..95d14dab089e 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > > @@ -191,7 +191,7 @@ static void intel_hdcp_info(struct seq_file *m,
> > > > struct intel_connector *intel_connector,
> > > > bool remote_req)
> > > > {
> > > > - bool hdcp_cap, hdcp2_cap;
> > > > + bool hdcp_cap = false, hdcp2_cap = false;
> > > >
> > > > if (!intel_connector->hdcp.shim) {
> > > > seq_puts(m, "No Connector Support");
> > > > --
> > > > 2.43.2
RE: [PATCH] drm/i915/display: Initalizalize capability variables
> -Original Message-
> From: Kandpal, Suraj
> Sent: Wednesday, March 27, 2024 9:06 AM
> To: Borah, Chaitanya Kumar ; intel-
> g...@lists.freedesktop.org
> Subject: RE: [PATCH] drm/i915/display: Initalizalize capability variables
>
> > Hello Suraj,
> >
> > > -Original Message-
> > > From: Kandpal, Suraj
> > > Sent: Tuesday, March 26, 2024 10:45 AM
> > > To: intel-gfx@lists.freedesktop.org
> > > Cc: Borah, Chaitanya Kumar ;
> > > Kandpal, Suraj
> > > Subject: [PATCH] drm/i915/display: Initalizalize capability
> > > variables
> >
> > Typo: Initialize
> >
>
> Sure will fix
>
> > > Initialize HDCP capability variables to false to avoid UBSAN warning
> > > in boolean value.
> > >
> >
> > I can see a case where hdcp_cap remains unassigned in case
> > intel_dp_hdcp_get_remote_capability() returns without assigning it a value.
> >
> > Is that intended/expected?
> >
>
> That is not intentional but this patch makes sure hdcp_capable will be false
> when this function gets called.
>
This patch makes sense for sure but what I am getting at is if UBSAN is
revealing a bug in our code.
intel_dp_hdcp_get_remote_capability() returns if
_intel_dp_hdcp2_get_capability() fails. In that case we never check for hdcp
1.4 capability.
If a remote sink can be hdcp 1.4 capable but not hdcp 2 capable, this logic is
problematic.
Regards
Chaitanya
> Regards,
> Suraj Kandpal
>
> > Regards
> >
> > Chaitanya
> >
> > > Signed-off-by: Suraj Kandpal
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > index b99c024b0934..95d14dab089e 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > @@ -191,7 +191,7 @@ static void intel_hdcp_info(struct seq_file *m,
> > > struct intel_connector *intel_connector,
> > > bool remote_req)
> > > {
> > > - bool hdcp_cap, hdcp2_cap;
> > > + bool hdcp_cap = false, hdcp2_cap = false;
> > >
> > > if (!intel_connector->hdcp.shim) {
> > > seq_puts(m, "No Connector Support");
> > > --
> > > 2.43.2
RE: [PATCH] drm/i915/display: Initalizalize capability variables
> Hello Suraj,
>
> > -Original Message-
> > From: Kandpal, Suraj
> > Sent: Tuesday, March 26, 2024 10:45 AM
> > To: intel-gfx@lists.freedesktop.org
> > Cc: Borah, Chaitanya Kumar ; Kandpal,
> > Suraj
> > Subject: [PATCH] drm/i915/display: Initalizalize capability variables
>
> Typo: Initialize
>
Sure will fix
> > Initialize HDCP capability variables to false to avoid UBSAN warning
> > in boolean value.
> >
>
> I can see a case where hdcp_cap remains unassigned in case
> intel_dp_hdcp_get_remote_capability() returns without assigning it a value.
>
> Is that intended/expected?
>
That is not intentional but this patch makes sure hdcp_capable will be false
when this function
gets called.
Regards,
Suraj Kandpal
> Regards
>
> Chaitanya
>
> > Signed-off-by: Suraj Kandpal
> > ---
> > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index b99c024b0934..95d14dab089e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -191,7 +191,7 @@ static void intel_hdcp_info(struct seq_file *m,
> > struct intel_connector *intel_connector,
> > bool remote_req)
> > {
> > - bool hdcp_cap, hdcp2_cap;
> > + bool hdcp_cap = false, hdcp2_cap = false;
> >
> > if (!intel_connector->hdcp.shim) {
> > seq_puts(m, "No Connector Support");
> > --
> > 2.43.2
RE: [PATCH] drm/i915/display: Initalizalize capability variables
Hello Suraj,
> -Original Message-
> From: Kandpal, Suraj
> Sent: Tuesday, March 26, 2024 10:45 AM
> To: intel-gfx@lists.freedesktop.org
> Cc: Borah, Chaitanya Kumar ; Kandpal,
> Suraj
> Subject: [PATCH] drm/i915/display: Initalizalize capability variables
Typo: Initialize
> Initialize HDCP capability variables to false to avoid UBSAN warning in
> boolean value.
>
I can see a case where hdcp_cap remains unassigned in case
intel_dp_hdcp_get_remote_capability() returns without assigning it a value.
Is that intended/expected?
Regards
Chaitanya
> Signed-off-by: Suraj Kandpal
> ---
> drivers/gpu/drm/i915/display/intel_display_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index b99c024b0934..95d14dab089e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -191,7 +191,7 @@ static void intel_hdcp_info(struct seq_file *m,
> struct intel_connector *intel_connector,
> bool remote_req)
> {
> - bool hdcp_cap, hdcp2_cap;
> + bool hdcp_cap = false, hdcp2_cap = false;
>
> if (!intel_connector->hdcp.shim) {
> seq_puts(m, "No Connector Support");
> --
> 2.43.2
