linux-kernel - RE: [char-misc-next V2] mei: hdcp: fix mei_hdcp_verify

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]
Message-ID: <feab8fb8fe1b4fcca385da217c6ba7a9@intel.com>
Date:   Thu, 30 Jul 2020 18:36:03 +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>,
        "C, Ramalingam" <ramalingam.c@...el.com>,
        "Gustavo A . R . Silva" <gustavoars@...nel.org>
Subject: RE: [char-misc-next V2] mei: hdcp: fix mei_hdcp_verify_mprime() input
 paramter

> Hi Tomas,
> 
> On 7/30/20 02:02, Tomas Winkler wrote:
> > wired_cmd_repeater_auth_stream_req_in has a variable length array at
> > the end. we use struct_size() overflow macro to determine the size for
> > the allocation and sending size.
> >
> 
> My comments here:
> https://lore.kernel.org/lkml/66c9950c-ef54-423e-467f-
> 38a9f7afb384@...eddedor.com/
Strange,  it haven't landed in my mailbox 


> still stand...


1. I don't think it should go to stable, true  there was bug but it wasn't triggered because there is only one stream.
2. In any case if we want to backport to stable, than all those fancy overflow macros are not there yet, need to rewrite the patch.
2. We cannot flex_array_size() as the streams in hdcp_port_data is not an array, I'm not going to change it in this patch. 

> 
> > Fixes: c56967d674e3 (mei: hdcp: Replace one-element array with
> > flexible-array member)
> > Fixes: c56967d674e3 (mei: hdcp: Replace one-element array with
> > flexible-array member)

> You have repeated the same commit twice, above.
Will fix 
 Also, a Fixes tag for the

> commit from Feb 2019 that I talk about here:
> https://lore.kernel.org/lkml/66c9950c-ef54-423e-467f-
> 38a9f7afb384@...eddedor.com/
Will add 
> should be included in this changelog text.
> 
> --
> Gustavo
> 
> > Cc: Ramalingam C <ramalingam.c@...el.com>
> > Cc: Gustavo A. R. Silva <gustavoars@...nel.org>
> > Signed-off-by: Tomas Winkler <tomas.winkler@...el.com>
> > ---
> > V2: Check for allocation failure.
> >  drivers/misc/mei/hdcp/mei_hdcp.c | 40
> > +++++++++++++++++++-------------
> >  1 file changed, 24 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c
> > b/drivers/misc/mei/hdcp/mei_hdcp.c
> > index d1d3e025ca0e..f1205e0060db 100644
> > --- a/drivers/misc/mei/hdcp/mei_hdcp.c
> > +++ b/drivers/misc/mei/hdcp/mei_hdcp.c
> > @@ -546,38 +546,46 @@ static int mei_hdcp_verify_mprime(struct device
> *dev,
> >  				  struct hdcp_port_data *data,
> >  				  struct hdcp2_rep_stream_ready
> *stream_ready)  {
> > -	struct wired_cmd_repeater_auth_stream_req_in
> > -					verify_mprime_in = { { 0 } };
> > +	struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
> >  	struct wired_cmd_repeater_auth_stream_req_out
> >  					verify_mprime_out = { { 0 } };
> >  	struct mei_cl_device *cldev;
> >  	ssize_t byte;
> > +	size_t cmd_size;
> >
> >  	if (!dev || !stream_ready || !data)
> >  		return -EINVAL;
> >
> >  	cldev = to_mei_cl_device(dev);
> >
> > -	verify_mprime_in.header.api_version = HDCP_API_VERSION;
> > -	verify_mprime_in.header.command_id =
> WIRED_REPEATER_AUTH_STREAM_REQ;
> > -	verify_mprime_in.header.status = ME_HDCP_STATUS_SUCCESS;
> > -	verify_mprime_in.header.buffer_len =
> > +	cmd_size = struct_size(verify_mprime_in, streams, data->k);
> > +	if (cmd_size == SIZE_MAX)
> > +		return -EINVAL;
> > +
> > +	verify_mprime_in = kzalloc(cmd_size, GFP_KERNEL);
> > +	if (!verify_mprime_in)
> > +		return -ENOMEM;
> > +
> > +	verify_mprime_in->header.api_version = HDCP_API_VERSION;
> > +	verify_mprime_in->header.command_id =
> WIRED_REPEATER_AUTH_STREAM_REQ;
> > +	verify_mprime_in->header.status = ME_HDCP_STATUS_SUCCESS;
> > +	verify_mprime_in->header.buffer_len =
> >
> 	WIRED_CMD_BUF_LEN_REPEATER_AUTH_STREAM_REQ_MIN_IN;
> >
> > -	verify_mprime_in.port.integrated_port_type = data->port_type;
> > -	verify_mprime_in.port.physical_port = (u8)data->fw_ddi;
> > -	verify_mprime_in.port.attached_transcoder = (u8)data->fw_tc;
> > +	verify_mprime_in->port.integrated_port_type = data->port_type;
> > +	verify_mprime_in->port.physical_port = (u8)data->fw_ddi;
> > +	verify_mprime_in->port.attached_transcoder = (u8)data->fw_tc;
> > +
> > +	memcpy(verify_mprime_in->m_prime, stream_ready->m_prime,
> HDCP_2_2_MPRIME_LEN);
> > +	drm_hdcp_cpu_to_be24(verify_mprime_in->seq_num_m, data-
> >seq_num_m);
> >
> > -	memcpy(verify_mprime_in.m_prime, stream_ready->m_prime,
> > -	       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,
> > +	memcpy(verify_mprime_in->streams, data->streams,
> >  	       array_size(data->k, sizeof(*data->streams)));
> >
> > -	verify_mprime_in.k = cpu_to_be16(data->k);
> > +	verify_mprime_in->k = cpu_to_be16(data->k);
> >
> > -	byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in,
> > -			      sizeof(verify_mprime_in));
> > +	byte = mei_cldev_send(cldev, (u8 *)&verify_mprime_in, cmd_size);
> > +	kfree(verify_mprime_in);
> >  	if (byte < 0) {
> >  		dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte);
> >  		return byte;
> >