From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists1p.gnu.org (lists1p.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C747BC531D0 for ; Mon, 27 Jul 2026 12:50:10 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists1p.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1woKmK-0006Il-7H; Mon, 27 Jul 2026 08:49:56 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists1p.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1woKmI-0006I2-AR for qemu-devel@nongnu.org; Mon, 27 Jul 2026 08:49:54 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1woKmG-000419-9B for qemu-devel@nongnu.org; Mon, 27 Jul 2026 08:49:54 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1785156591; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Hgy/nJHeUav8gNp8gzof/zmVhbZTz7N4ugaONbvOehA=; b=ClOdDle4xOYR+TvtOUC6WwExVeYUCm5NKoKsBFEfSQEz9oolsRWRwh8K+VjliQSMOQRNVu WzfenAMTp0IuT9BjHCWf9hdFV2Hb58Ke96IQJoYZZKFQxKWu413tZCpVO70xInKQ4U3+XH qNufL0H0y+wnam9ucQjfmZoNRNoYx7s= Received: from mail-qv1-f72.google.com (mail-qv1-f72.google.com [209.85.219.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-638-UFnX-b0ROoC7O5-Gk1Js3g-1; Mon, 27 Jul 2026 08:49:50 -0400 X-MC-Unique: UFnX-b0ROoC7O5-Gk1Js3g-1 X-Mimecast-MFC-AGG-ID: UFnX-b0ROoC7O5-Gk1Js3g_1785156589 Received: by mail-qv1-f72.google.com with SMTP id 6a1803df08f44-8eeba1d9e47so43374846d6.2 for ; Mon, 27 Jul 2026 05:49:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1785156589; x=1785761389; darn=nongnu.org; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:from:to:cc:subject:date:message-id:reply-to:content-type; bh=Hgy/nJHeUav8gNp8gzof/zmVhbZTz7N4ugaONbvOehA=; b=OvXpgEOfXyUhLR1CBk8s8e2Lx5s+669dsVCMhpdiXNOd9MJK0+NOWhUHutlqZNxcq8 nPUKzVhpZZSKVa+MjkRzKSoQM7LaCQoAeI6GRfWh914chsS3/y5E/eNI3hTUqj5+fZAs 5xDucBX6ZIjE6DjSke1oREJX+hKWcrKSqO6fHQBkrjJax0nE0TNO3g5m8eMeFSSi++3C j4QMxUdscGZNwO/BiO+3AckboqpK7M+24JkBDS4j09uW2nuq2H76sMVDRJ4osnedUPxj uvy9ovMSr+koWJVcLvYtQgny5DDzPZZiv3lXCRBCqwRuBUHA8hOqIBPqlLiWdKpALE5E hXBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1785156589; x=1785761389; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:message-id:subject:cc:to:from :date:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to:content-type; bh=Hgy/nJHeUav8gNp8gzof/zmVhbZTz7N4ugaONbvOehA=; b=CFeh6g76NfCYu7RgT/JTo4luwBZ5whukfkqld1mQ9JrnlaTZYYKRloxuYTImrEJDpT Smhs1aX+6TORb59I/wPjKbsWkJcWu96DwI9Wy2Xx1R2Ng6zoYmxqwbp6wAXYmeTWervW PGQTr9DwAroNKWE5NRVAfmRIotySzagm9sCTaByxf/3Dp5uVx82YhSnh1pDYWnSZgJbm jISKS7E04bs3yqX5JjFk0tKCbgn5nL+p2M6erhnHDcTVSZ0Jp+YYwP6zVLuIg3OGvtDN 1c95SWjgEJnbofy1423aDnHFfGwRt+8Nc5n8cO1yAHz53MOzWT66u/CChoYje4qrg9KM k8Kg== X-Gm-Message-State: AOJu0Yx2E15Jl/CYUEk84PNDrEfk4stPrvPheJB4E963qfFJthDlkBg3 CIvxX6PyAbMLKJpwlCc7jVLn516JVbEqdZ7p3vV8ylML6aECDXZczr5HhpDSG3aNPeDMl5sT+g3 GZ52wmqDyTYa1w3n9Oy/Q4jiEkPOQiYr8yp1nbpCDDww0WcWDuH4K9kLj X-Gm-Gg: AR+sD13/LLUjP9NqxyhrIF830/2Upe9B29vR4b6Fg3FZuTMhrpG3gC3bWZ2iZf/U/q4 FJBXvQf/JfCncH/Hmp6pmUYwJydb02QSZLxECK7AWbwQTeRMkIedHo+wlvCXMOs6ReZMdk1MJ0f gZ2QFbAFKyvnF4uRK8Rw4lzLy4aGEvTekLhXnGpLtEAg+seHe2nP1QKDRXq7+VN2kgWVR9lXeSp 9LzHBXeS7guLwoQJPleDRe0lJyuuyCtkMHS9TQNgB1wyvkkYxMizKZY5czMy+wUF+S9vasj+M4a u9ZSIWy/gtzHEX4X1Fa7mm+Bwmg8WsPlNurhxZsM/YtgtWsVnKeXAYfAOLkAYXIkyMlF X-Received: by 2002:a05:6214:20a1:b0:8f7:40b8:e8ab with SMTP id 6a1803df08f44-907ec7959cemr106503256d6.3.1785156589199; Mon, 27 Jul 2026 05:49:49 -0700 (PDT) X-Received: by 2002:a05:6214:20a1:b0:8f7:40b8:e8ab with SMTP id 6a1803df08f44-907ec7959cemr106502906d6.3.1785156588609; Mon, 27 Jul 2026 05:49:48 -0700 (PDT) Received: from x1.local ([174.91.117.74]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-907e854e43csm64462836d6.16.2026.07.27.05.49.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 27 Jul 2026 05:49:47 -0700 (PDT) Date: Mon, 27 Jul 2026 08:49:44 -0400 From: Peter Xu To: "Michael S. Tsirkin" Cc: QEMU Developers , Peter Maydell , Alexandr Moshkov , Fabiano Rosas Subject: Re: [PULL 17/30] vmstate: fix type confusion in vmstate_size() for VMSTATE_VBUFFER_UINT64 Message-ID: References: <9e2d0b03a60642236e6df8edde7b3562f5f9849f.1785101237.git.mst@redhat.com> <20260727045220-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260727045220-mutt-send-email-mst@kernel.org> Received-SPF: pass client-ip=170.10.133.124; envelope-from=peterx@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -36 X-Spam_score: -3.7 X-Spam_bar: --- X-Spam_report: (-3.7 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-1.58, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: qemu development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org On Mon, Jul 27, 2026 at 04:53:15AM -0400, Michael S. Tsirkin wrote: > On Sun, Jul 26, 2026 at 08:31:57PM -0400, Peter Xu wrote: > > This patch is migration only change and hasn't been reviewed.  Give us a few > > days to review it? > > Can't do it now because it is on cellphone and I just bathed my son. > > > > Please hold off merging. > > > > Thanks. > > OK. I'll drop this and you will merge it through the migration tree > as appropriate? Let's drop it first so it won't block this pull. I can definitely pick it up when ready, but I want to discuss on how to fix first. Which tree to pick it up isn't a huge matter to me, but the review. We found a bunch of randomly introduced VMSTATE flags in the past, I used to remove some, at least my plan is in the future each time we introduce a new one (by request from other modules) we better justify it before landing too easily, in case it needs a revert again. This vhost regression started 11.0 so IIUC we don't need to rush in 11.1. I just noticed I read that patch and didn't notice this, my bad to not have noticed.. For this one specifically.. > > > On Sun, Jul 26, 2026, 5:29 p.m. Michael S. Tsirkin wrote: > > > > vhost user currently saves the inflight buffer to the migration stream > > using VMSTATE_VBUFFER_UINT64. The size is controlled by the vhost-user > > backend. > > > > But the implementation of that is broken if size is >2G: it stores the > > buffer size in a uint64_t field, but vmstate_size() always reads the > > size field as int32_t regardless of the macro used. This, in turn, > > causes negative or truncated lengths on load, leading to undersized > > allocations and down the road out-of-bounds buffer access. > > > > There's no practical reason to support such large sizes, so it's enough If there's no real demand to use 2G or more, I suggest we change size to int32_t instead on the user, and revert f6fdd8b2 VMSTATE_VBUFFER_UINT64(). Fabiano and I were looking at rest 20+ possible security related bugs in the past 1-2 weeks, I've some patches to be posted for 11.2 too when we went through all of them. One relevant patch I haven't sent but will do soon: https://gitlab.com/peterx/qemu/-/commit/afd2cb25defdf78a4ab5279cc16634884792c57a We encountered quite a few of possible over-allocation on destionation side by manipulating on-wire length field like this one, I believe Fabiano is looking at how to further limit that from 2G if ever possible per-user. I'm not sure if that idea will fly, but I think anything bigger than 2G definitely is not suggested for now when it's only for a type match not real demand.. I believe VMSTATE_VBUFFER_UINT32 is problematic on its own too, I'll see how to fix that. That one is slightly easier, worst case is we bail out for 2G (hence ignore bit 31, which shouldn't be used in reality for legit users), but I'll think about it. Thanks, > > to validate: read the field as uint64_t when VMS_VBUFFER_UINT64 is set, > > reject negative or oversized values, and propagate errors to > > vmstate_load_vmsd(). > > > > Note: the large value is coming from the backend, not guest, so this > > shouldn't be considered a security issue. The CVE was assigned before > > the qemu security policy was updated to exclude this class of bugs. > > > > Fixes: CVE-2026-6426 > > Fixes: f6fdd8b2bd ("vmstate: introduce VMSTATE_VBUFFER_UINT64") > > Cc: Alexandr Moshkov > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3675 > > Signed-off-by: Michael S. Tsirkin > > Message-ID: < > > 801b1501ee10241f7ac49a10570d548352b36347.1784890517.git.mst@redhat.com> > > --- > >  include/migration/vmstate.h |  5 ++++- > >  migration/vmstate.c         | 31 ++++++++++++++++++++++++++++--- > >  2 files changed, 32 insertions(+), 4 deletions(-) > > > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > > index 1b7f295417..e7095cd977 100644 > > --- a/include/migration/vmstate.h > > +++ b/include/migration/vmstate.h > > @@ -168,6 +168,9 @@ enum VMStateFlags { > >       */ > >      VMS_ARRAY_OF_POINTER_AUTO_ALLOC = 0x10000, > > > > +    /* Use a uint64_t size field for VMS_VBUFFER instead of int32_t. */ > > +    VMS_VBUFFER_UINT64              = 0x40000, > > + > >      /* Marker for end of list */ > >      VMS_END                         = 0x20000, > >  }; > > @@ -788,7 +791,7 @@ extern const VMStateInfo vmstate_info_g_byte_array; > >      .field_exists = (_test),                                         \ > >      .size_offset  = vmstate_offset_value(_state, _field_size, uint64_t),\ > >      .info         = &vmstate_info_buffer,                            \ > > -    .flags        = VMS_VBUFFER | VMS_POINTER,                       \ > > +    .flags        = VMS_VBUFFER | VMS_VBUFFER_UINT64 | VMS_POINTER,  \ > >      .offset       = offsetof(_state, _field),                        \ > >  } > > > > diff --git a/migration/vmstate.c b/migration/vmstate.c > > index 50ebe37845..d7f03a9f5b 100644 > > --- a/migration/vmstate.c > > +++ b/migration/vmstate.c > > @@ -103,10 +103,24 @@ static int vmstate_size(void *opaque, const > > VMStateField *field) > >      int size; > > > >      if (field->flags & VMS_VBUFFER) { > > -        size = *(int32_t *)(opaque + field->size_offset); > > -        if (field->flags & VMS_MULTIPLY) { > > -            size *= field->size; > > +        uint64_t usize64; > > + > > +        if (field->flags & VMS_VBUFFER_UINT64) { > > +            usize64 = *(uint64_t *)(opaque + field->size_offset); > > +        } else { > > +            int32_t ssize32 = *(int32_t *)(opaque + field->size_offset); > > +            if (ssize32 < 0) { > > +                return -1; > > +            } > > +            usize64 = ssize32; > >          } > > +        if (field->flags & VMS_MULTIPLY) { > > +            usize64 *= field->size; > > +        } > > +        if (usize64 > INT_MAX) { > > +            return -1; > > +        } > > +        size = usize64; > >      } else if (field->flags & VMS_ARRAY_OF_POINTER) { > >          /* > >           * For an array of pointer, the each element is always size of a > > @@ -337,6 +351,11 @@ bool vmstate_load_vmsd(QEMUFile *f, const > > VMStateDescription *vmsd, > >              void *first_elem = opaque + field->offset; > >              int i, n_elems = vmstate_n_elems(opaque, field); > >              int size = vmstate_size(opaque, field); > > +            if (size < 0) { > > +                error_setg(errp, "VMState field '%s': invalid size", > > +                           field->name); > > +                return false; > > +            } > > > >              vmstate_handle_alloc(first_elem, field, opaque); > >              if (field->flags & VMS_POINTER) { > > @@ -661,6 +680,12 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const > > VMStateDescription *vmsd, > >              bool use_dynamic_array = > >                  field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC; > > > > +            if (size < 0) { > > +                error_setg(errp, "VMState field '%s': invalid size", > > +                           field->name); > > +                ok = false; > > +                goto out; > > +            } > >              trace_vmstate_save_state_loop(vmsd->name, field->name, > > n_elems); > >              if (field->flags & VMS_POINTER) { > >                  first_elem = *(void **)first_elem; > > -- > > MST > > > > > -- Peter Xu