linux-kernel - Re: [PATCH v4 5/7] platform/chrome: cros_ec

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: <2bfe74d6a29ca13a7f89f116a2f0c6be.sboyd@kernel.org>
Date: Wed, 11 Dec 2024 13:58:30 -0800
From: Stephen Boyd <sboyd@...nel.org>
To: Abhishek Pandit-Subedi <abhishekpandit@...omium.org>, chrome-platform@...ts.linux.dev, heikki.krogerus@...ux.intel.com, linux-usb@...r.kernel.org, tzungbi@...nel.org
Cc: akuchynski@...gle.com, pmalani@...omium.org, jthies@...gle.com, dmitry.baryshkov@...aro.org, badhri@...gle.com, rdbabiera@...gle.com, Abhishek Pandit-Subedi <abhishekpandit@...omium.org>, Benson Leung <bleung@...omium.org>, Guenter Roeck <groeck@...omium.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 5/7] platform/chrome: cros_ec_typec: Displayport support

Quoting Abhishek Pandit-Subedi (2024-12-06 15:38:16)
> diff --git a/drivers/platform/chrome/cros_typec_altmode.c b/drivers/platform/chrome/cros_typec_altmode.c
> new file mode 100644
> index 000000000000..bb7c7ad2ff6e
> --- /dev/null
> +++ b/drivers/platform/chrome/cros_typec_altmode.c
> @@ -0,0 +1,281 @@
[...]
> +
> +static const struct typec_altmode_ops cros_typec_altmode_ops = {
> +       .enter = cros_typec_altmode_enter,
> +       .exit = cros_typec_altmode_exit,
> +       .vdm = cros_typec_altmode_vdm,
> +};
> +
> +#if IS_ENABLED(CONFIG_TYPEC_DP_ALTMODE)
> +int cros_typec_displayport_status_update(struct typec_altmode *altmode,
> +                                        struct typec_displayport_data *data)
> +{
> +       struct cros_typec_dp_data *dp_data =
> +               typec_altmode_get_drvdata(altmode);

How does this work? I see that the type of the drvdata is struct
cros_typec_altmode_data per the allocation in
cros_typec_register_displayport(), but here we're treating it as the
type struct cros_typec_dp_data, which has a struct
cros_typec_altmode_data as the first member. The allocation is too small
from what I can tell. The same problem looks to be there in
cros_typec_displayport_vdm().

> +       struct cros_typec_altmode_data *adata = &dp_data->adata;
> +
> +       if (!dp_data->pending_status_update) {
> +               dev_dbg(&altmode->dev,
> +                       "Got DPStatus without a pending request");
> +               return 0;
> +       }
> +
> +       if (dp_data->configured && dp_data->data.conf != data->conf)
> +               dev_dbg(&altmode->dev,
> +                       "DP Conf doesn't match. Requested 0x%04x, Actual 0x%04x",
> +                       dp_data->data.conf, data->conf);
> +
> +       mutex_lock(&adata->lock);
> +
> +       dp_data->data = *data;
> +       dp_data->pending_status_update = false;
> +       adata->header |= VDO_CMDT(CMDT_RSP_ACK);
> +       adata->vdo_data = &dp_data->data.status;
> +       adata->vdo_size = 2;
> +       schedule_work(&adata->work);
> +
> +       mutex_unlock(&adata->lock);
> +
> +       return 0;
> +}
> +
> +struct typec_altmode *
> +cros_typec_register_displayport(struct cros_typec_port *port,
> +                               struct typec_altmode_desc *desc,
> +                               bool ap_mode_entry)
> +{
> +       struct typec_altmode *alt;
> +       struct cros_typec_altmode_data *data;
> +
> +       alt = typec_port_register_altmode(port->port, desc);
> +       if (IS_ERR(alt))
> +               return alt;
> +
> +       data = devm_kzalloc(&alt->dev, sizeof(*data), GFP_KERNEL);
> +       if (!data) {
> +               typec_unregister_altmode(alt);
> +               return ERR_PTR(-ENOMEM);
> +       }
> +
> +       INIT_WORK(&data->work, cros_typec_altmode_work);
> +       mutex_init(&data->lock);
> +       data->alt = alt;
> +       data->port = port;
> +       data->ap_mode_entry = ap_mode_entry;
> +       data->sid = desc->svid;
> +       data->mode = desc->mode;
> +
> +       typec_altmode_set_ops(alt, &cros_typec_altmode_ops);
> +       typec_altmode_set_drvdata(alt, data);

'data' is of type struct cros_typec_altmode_data here

> +
> +       return alt;
> +}