netdev - Re: [PATCH v2 05/14] net: wwan: t7xx: Add control port

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: <CAHNKnsTZhtmt6DNsEb+EvnFBo9mNPd4pPG=qjvkxyD3pH+KqrA@mail.gmail.com>
Date:   Sat, 6 Nov 2021 21:07:03 +0300
From:   Sergey Ryazanov <ryazanov.s.a@...il.com>
To:     Ricardo Martinez <ricardo.martinez@...ux.intel.com>
Cc:     netdev@...r.kernel.org, linux-wireless@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>,
        Johannes Berg <johannes@...solutions.net>,
        Loic Poulain <loic.poulain@...aro.org>,
        M Chetan Kumar <m.chetan.kumar@...el.com>,
        chandrashekar.devegowda@...el.com,
        Intel Corporation <linuxwwan@...el.com>,
        chiranjeevi.rapolu@...ux.intel.com, haijun.liu@...iatek.com,
        amir.hanania@...el.com,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        dinesh.sharma@...el.com, eliot.lee@...el.com,
        mika.westerberg@...ux.intel.com, moises.veleta@...el.com,
        pierre-louis.bossart@...el.com, muralidharan.sethuraman@...el.com,
        Soumya.Prakash.Mishra@...el.com, sreehari.kancharla@...el.com,
        suresh.nagaraj@...el.com
Subject: Re: [PATCH v2 05/14] net: wwan: t7xx: Add control port

On Mon, Nov 1, 2021 at 6:57 AM Ricardo Martinez
<ricardo.martinez@...ux.intel.com> wrote:
> Control Port implements driver control messages such as modem-host
> handshaking, controls port enumeration, and handles exception messages.
>
> The handshaking process between the driver and the modem happens during
> the init sequence. The process involves the exchange of a list of
> supported runtime features to make sure that modem and host are ready
> to provide proper feature lists including port enumeration. Further
> features can be enabled and controlled in this handshaking process.

[skipped]

> diff --git a/drivers/net/wwan/t7xx/t7xx_modem_ops.c b/drivers/net/wwan/t7xx/t7xx_modem_ops.c
> ...
> +struct feature_query {
> +       u32 head_pattern;

Looks like this field should be __le32 since it is sent to modem as is.

> +       u8 feature_set[FEATURE_COUNT];
> +       u32 tail_pattern;

Ditto.

> +};
> +
> +static void prepare_host_rt_data_query(struct core_sys_info *core)
> +{
> ...
> +       ft_query->head_pattern = MD_FEATURE_QUERY_ID;

This should be a

  ft_query->head_pattern = cpu_to_le32(MD_FEATURE_QUERY_ID);

to run on the big-endians CPU. strace will notify you about each
endians mismatch as soon as you change head_pattern field type to
__le32.

> +       memcpy(ft_query->feature_set, core->feature_set, FEATURE_COUNT);
> +       ft_query->tail_pattern = MD_FEATURE_QUERY_ID;

Ditto.

> +       /* send HS1 message to device */
> +       port_proxy_send_skb(core->ctl_port, skb, 0);
> +}

[skipped]

> diff --git a/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c b/drivers/net/wwan/t7xx/t7xx_port_ctrl_msg.c
> ...
> +static void fsm_ee_message_handler(struct sk_buff *skb)
> +{
> ...
> +       ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
> ...
> +       switch (ctrl_msg_h->ctrl_msg_id) {

This should be:

        switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) {

> +       case CTL_ID_MD_EX:
> +               if (ctrl_msg_h->reserved != MD_EX_CHK_ID) {

Why is this field called 'reserved', but used to perform message validation?

> ...
> +static void control_msg_handler(struct t7xx_port *port, struct sk_buff *skb)
> +{
> +       struct ctrl_msg_header *ctrl_msg_h;
> ...
> +       ctrl_msg_h = (struct ctrl_msg_header *)skb->data;
> ..
> +       switch (ctrl_msg_h->ctrl_msg_id) {

This should be something like this:

        switch (le32_to_cpu(ctrl_msg_h->ctrl_msg_id)) {

--
Sergey