On 27/8/23 13:07, Mauro Matteo Cascella wrote:
On Sat, Aug 26, 2023 at 4:31 PM Mauro Matteo Cascella
<mcasc...@redhat.com> wrote:
On Fri, Jun 24, 2022 at 4:40 PM Mauro Matteo Cascella
<mcasc...@redhat.com> wrote:
rocker_tlv_parse_nested could return early because of no group ids in
the group_tlvs. In such case tlvs is NULL; tlvs[i + 1] in the next
for-loop will deref the NULL pointer.
Looking at the code once again, tlvs is a pointer to a g_new0
allocated memory, so I don't know how it can be NULL after
rocker_tlv_parse_nested (unless g_new0 returns NULL in the first
place). I do not recall the details of this bug. Arayz?
Per <glib.h>:
If any call to allocate memory using functions g_new(), g_new0(),
g_renew(), g_malloc(), g_malloc0(), g_malloc0_n(), g_realloc(), and
g_realloc_n() fails, the application is terminated. This also means
that there is no need to check if the call succeeded. On the other
hand, g_try_...() family of functions returns NULL on failure that
can be used as a check for unsuccessful memory allocation. The
application is not terminated in this case.
group->l2_flood.group_count is a uint16_t, so up to UINT16_MAX = 0xffff.
So:
tlvs = g_new0(RockerTlv *, group->l2_flood.group_count + 1);
is at most an malloc(0x10000 * sizeof(void *)) = 0x80000 = 512 KiB.
QEMU use way bigger heap allocations.
I don't know the net/ subsystem enough to have an idea about how many
concurrent packets can be processed by this device model, but I'd be
surprised if this triggers a OOM.
As usual, do you have a reproducer?
Someone somehow reserved a new CVE for this bug, published a few days
ago here: https://nvd.nist.gov/vuln/detail/CVE-2022-36648.
Not only is this not CVE worthy (rocker code does not fall under the
KVM virtualization use case [1]) but what's most concerning is that it
got a CVSS score of 10 :/
I'm going to dispute this CVE. Hopefully, it will be rejected soon. In
any case, can we get this patch merged?
[1] https://www.qemu.org/docs/master/system/security.html
Thanks,
Signed-off-by: Mauro Matteo Cascella <mcasc...@redhat.com>
Reported-by: <aray...@icloud.com>
---
hw/net/rocker/rocker_of_dpa.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index b3b8c5bb6d..1611b79227 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -2039,6 +2039,11 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa,
OfDpaGroup *group,
rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
+ if (!tlvs) {
+ err = -ROCKER_EINVAL;
+ goto err_out;
+ }
+
for (i = 0; i < group->l2_flood.group_count; i++) {
group->l2_flood.group_ids[i] = rocker_tlv_get_le32(tlvs[i + 1]);
}
--
2.35.3
