linux-kernel - RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array with flexible-array member"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 Jul 2020 22:01:02 +0000
From:   "Winkler, Tomas" <tomas.winkler@...el.com>
To:     "Gustavo A. R. Silva" <gustavo@...eddedor.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
CC:     "Usyskin, Alexander" <alexander.usyskin@...el.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Gustavo A . R . Silva" <gustavoars@...nel.org>
Subject: RE: [char-misc-next] Revert "mei: hdcp: Replace one-element array
 with flexible-array member"



> -----Original Message-----
> From: Gustavo A. R. Silva <gustavo@...eddedor.com>
> Sent: Wednesday, July 29, 2020 00:57
> To: Winkler, Tomas <tomas.winkler@...el.com>; Greg Kroah-Hartman
> <gregkh@...uxfoundation.org>
> Cc: Usyskin, Alexander <alexander.usyskin@...el.com>; linux-
> kernel@...r.kernel.org; Gustavo A . R . Silva <gustavoars@...nel.org>
> Subject: Re: [char-misc-next] Revert "mei: hdcp: Replace one-element array
> with flexible-array member"
> 
> Hi Tomas,
> 
> On 7/28/20 16:41, Tomas Winkler wrote:
> > Greg please revert, this commit it changes size of struct
> > wired_cmd_repeater_auth_stream_req_in, this is not what firmware is
> > expecting.
> 
> Could you elaborate on what's the firmware expecting, exactly?
struct wired_cmd_repeater_auth_stream_req_in {
  -	struct hdcp2_streamid_type	streams[1];
  + 	struct hdcp2_streamid_type	streams[];
}

But then you have, which you haven't changed to + 1
byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in,
                              sizeof(verify_mprime_in));

But that's not the major point. Point is that we should be able to review
and test the code before it is merged.  You haven't run it, right? 
There is MAINTAINERS file for a reason. 


> 
> Thanks
> --
> Gustavo
> 
> > I really do not appreciate that the code is bypassing driver maintaner
> > review, I think this is a minimum we can ask for, this is not for a
> > first time.
> >
> > This reverts commit c56967d674e361ebe716e66992e3c5332b25ac1f.
> >
> > Cc: Gustavo A. R. Silva <gustavoars@...nel.org>
> > Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> > ---
> >  drivers/misc/mei/hdcp/mei_hdcp.c | 2 +-
> > drivers/misc/mei/hdcp/mei_hdcp.h | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> > b/drivers/misc/mei/hdcp/mei_hdcp.c
> > index d1d3e025ca0e..e6c3dc595617 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> > @@ -572,7 +572,7 @@ static int mei_hdcp_verify_mprime(struct device
> *dev,
> >  	       HDCP_2_2_MPRIME_LEN);
> >  	drm_hdcp_cpu_to_be24(verify_mprime_in.seq_num_m, data-
> >seq_num_m);
> >  	memcpy(verify_mprime_in.streams, data->streams,
> > -	       array_size(data->k, sizeof(*data->streams)));
> > +	       (data->k * sizeof(struct hdcp2_streamid_type)));
> >
> >  	verify_mprime_in.k = cpu_to_be16(data->k);
> >
> > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.h
> > b/drivers/misc/mei/hdcp/mei_hdcp.h
> > index 834757f5e072..18ffc773fa18 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.h
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.h
> > @@ -358,7 +358,7 @@ struct wired_cmd_repeater_auth_stream_req_in {
> >  	u8
> 	seq_num_m[HDCP_2_2_SEQ_NUM_LEN];
> >  	u8				m_prime[HDCP_2_2_MPRIME_LEN];
> >  	__be16				k;
> > -	struct hdcp2_streamid_type	streams[];
> > +	struct hdcp2_streamid_type	streams[1];
> >  } __packed;
> >
> >  struct wired_cmd_repeater_auth_stream_req_out {
> >