linux-kernel - out-of-bounds access in sound/soc/sof/topology.c

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 for Android: free password hash cracker in your pocket

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

Message-ID: <Ylk5o3EC/hyX5UQ0@google.com>
Date:   Fri, 15 Apr 2022 18:23:47 +0900
From:   Sergey Senozhatsky <senozhatsky@...omium.org>
To:     Liam Girdwood <liam.r.girdwood@...ux.intel.com>,
        Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Kai Vehmanen <kai.vehmanen@...ux.intel.com>
Cc:     Takashi Iwai <tiwai@...e.com>, Jaroslav Kysela <perex@...ex.cz>,
        Mark Brown <broonie@...nel.org>,
        Ricardo Ribalda <ribalda@...omium.org>,
        Tomasz Figa <tfiga@...omium.org>,
        sound-open-firmware@...a-project.org, alsa-devel@...a-project.org,
        linux-kernel@...r.kernel.org
Subject: out-of-bounds access in sound/soc/sof/topology.c

Hi,

I'm running 5.10.111 LTS, so if this has been fixed already then we definitely
want to cherry pick the fix for -stable.


Anonymous union in this struct is of zero size

/* generic control data */
struct sof_ipc_ctrl_data {
        struct sof_ipc_reply rhdr;
        uint32_t comp_id;

        /* control access and data type */
        uint32_t type;          /**< enum sof_ipc_ctrl_type */
        uint32_t cmd;           /**< enum sof_ipc_ctrl_cmd */
        uint32_t index;         /**< control index for comps > 1 control */

        /* control data - can either be appended or DMAed from host */
        struct sof_ipc_host_buffer buffer;
        uint32_t num_elems;     /**< in array elems or bytes for data type */
        uint32_t elems_remaining;       /**< elems remaining if sent in parts */

        uint32_t msg_index;     /**< for large messages sent in parts */

        /* reserved for future use */
        uint32_t reserved[6];

        /* control data - add new types if needed */
        union {
                /* channel values can be used by volume type controls */
                struct sof_ipc_ctrl_value_chan chanv[0];
                /* component values used by routing controls like mux, mixer */
                struct sof_ipc_ctrl_value_comp compv[0];
                /* data can be used by binary controls */
                struct sof_abi_hdr data[0];
        };
} __packed;

sof_ipc_ctrl_value_chan and sof_ipc_ctrl_value_comp are of the same
size - 8 bytes, while sof_abi_hdr is much larger - _at least_ 32 bytes
(`__u32 data[0]` in sof_abi_hdr suggest that there should be more
payload after header). But they all contribute 0 to sizeof(sof_ipc_ctrl_data).

Now control data allocations looks as follows

	scontrol->size = struct_size(scontrol->control_data, chanv,
				     le32_to_cpu(mc->num_channels));
	scontrol->control_data = kzalloc(scontrol->size, GFP_KERNEL);

Which is sizeof(sof_ipc_ctrl_data) + mc->num_channels * sizeof(sof_ipc_ctrl_value_chan)

For some reason it uses sizeof(sof_ipc_ctrl_value_chan), which is not
the largest member of the union.

And this is where the problem is: in order to make control->data.FOO loads
and stores legal we need mc->num_channels to be of at least 4. So that

   sizeof(sof_ipc_ctrl_data) + mc->num_channels * sizeof(sof_ipc_ctrl_value_chan)

                92           +        4         *            8

will be the same as

   sizeof(sof_ipc_ctrl_data) + sizeof(sof_abi_hdr).

                92           +           32

Otherwise scontrol->control_data->data.FOO will access nearby/foreign
slab object.

And there is at least one such memory access. In sof_get_control_data().

	wdata[i].pdata = wdata[i].control->control_data->data;
	*size += wdata[i].pdata->size;

pdata->size is at offset 8, but if, say, mc->num_channels == 1 then
we allocate only 8 bytes for pdata, so pdata->size is 4 bytes outside
of allocated slab object.

Thoughts?