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 X-Spam-Level: X-Spam-Status: No, score=-7.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 4C2CEC433E0 for ; Mon, 15 Mar 2021 19:19:15 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A2AAF64F46 for ; Mon, 15 Mar 2021 19:19:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A2AAF64F46 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:43700 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lLskD-0006b1-KJ for qemu-devel@archiver.kernel.org; Mon, 15 Mar 2021 15:19:13 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:50942) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lLsJe-0006NI-I5 for qemu-devel@nongnu.org; Mon, 15 Mar 2021 14:51:46 -0400 Received: from mail-ed1-x52a.google.com ([2a00:1450:4864:20::52a]:47035) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lLsJa-00042m-Pl for qemu-devel@nongnu.org; Mon, 15 Mar 2021 14:51:46 -0400 Received: by mail-ed1-x52a.google.com with SMTP id h10so18555866edt.13 for ; Mon, 15 Mar 2021 11:51:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8hGa1WZT3IRVbcAr8nby2MZF2hWby7QFZIvGcz/+QNA=; b=MKouHrIDm1oEm0PaXZnHgq4G59np6o5gkHDXmsj5kCPIgEoqfQ3k1aQI1qg7Z+eyV5 JdF70j7efFiDd496aqMzY9QuOQJb7orIGMGdyMrCH+VSs53QEaDjLoytWpp/jli1TEyA IOA5Da2jWoVaUEGl18AnraqR91/Gp54Phn9jiUEO42UyRqY2F228EEaQRwuqgpk4oru7 rAe4Njg244/w6wOMmSlOuKfBvOwJ2RQDkCv9qyOZ35MuGVl2bP1aM+947SjlxmcIMFO9 wqHurbpWKgHVTstbSF5otpWkzDAkRN/whg8wHJnGwiJMotLyv9yLiDXeKyhWamuYWP6O lurA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8hGa1WZT3IRVbcAr8nby2MZF2hWby7QFZIvGcz/+QNA=; b=Il2z739I1Oe5MM+Ev1tdpVsZuYOn6DEoOI/KACTn3CCJWEFWKmjLPODbYim7zQdfRv f6XVrH9+SiNgOp9GvqUAfD3Vsf+DabvJZi21e043IzyKuxFxl8y0nMalMTx2i7ZE0HTU NoWP8mIpFTtKSGReCf6BhPS0q5/gPe7ZSrw8sQNAMSn8OwO+a+hwJjmPlIxMersVjfR9 tH2eWaeVG2rMkqVnRtLt83AL4kmjSujhMwHQLTr4fuGJ4ObsKJXEIHVjazz5PT7lnz97 kL3R9mLoOO+DSs2LgPtsmpz9fvtjZYWuQzLjv3crs7ow+6USt4LktgVth5Eog9NVtKGU Ivng== X-Gm-Message-State: AOAM530y3ohFkEDJMqBsIQGFKUKkjkP8LVmb231MhlWs6uyWhR1Tb6Dd sf5n9cANrwM5Sj6XrfRFqWf0zPM5kzWNEJnlL6I= X-Google-Smtp-Source: ABdhPJzmHb0AALIWw5i/c/QI8IaVbzx4ISlpHiYGBK8fpyAUGjp54bOnPgPdyXKU3fYfDjYPKxRV9VVQajYvjAGGzi8= X-Received: by 2002:a50:ee05:: with SMTP id g5mr31543839eds.164.1615834300632; Mon, 15 Mar 2021 11:51:40 -0700 (PDT) MIME-Version: 1.0 References: <20210315170636.704201-1-zhlcindy@gmail.com> In-Reply-To: <20210315170636.704201-1-zhlcindy@gmail.com> From: =?UTF-8?B?TWFyYy1BbmRyw6kgTHVyZWF1?= Date: Mon, 15 Mar 2021 22:51:27 +0400 Message-ID: Subject: Re: [PATCH 1/2] Fix the segment fault when calling yank_register_instance To: Li Zhang Content-Type: multipart/alternative; boundary="00000000000081529d05bd97bd9c" Received-SPF: pass client-ip=2a00:1450:4864:20::52a; envelope-from=marcandre.lureau@gmail.com; helo=mail-ed1-x52a.google.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=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.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lukas Straub , alexandr.iarygin@profitbricks.com, Markus Armbruster , QEMU , Li Zhang , pankaj.gupta@cloud.ionos.com Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --00000000000081529d05bd97bd9c Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi On Mon, Mar 15, 2021 at 9:22 PM Li Zhang wrote: > From: Li Zhang > > When executing the QMP commands "chardev-change" to change the > backend device to socket, it will cause a segment fault because > it assumes chr->label as non-NULL in function yank_register_instance. > The function qmp_chardev_change calls chardev_new, which label > is NULL when creating a new chardev. The label will be passed to > yank_register_instance which causes a segment fault. The callchain > is as the following: > chardev_new -> > qemu_char_open -> > cc->open -> > qmp_chardev_open_socket -> > yank_register_instance > > Signed-off-by: Li Zhang > --- > chardev/char-socket.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/chardev/char-socket.c b/chardev/char-socket.c > index c8bced76b7..26d5172682 100644 > --- a/chardev/char-socket.c > +++ b/chardev/char-socket.c > @@ -1421,10 +1421,12 @@ static void qmp_chardev_open_socket(Chardev *chr, > qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS); > } > > - if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)= ) > { > - return; > + if (chr->label) { > + if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), > errp)) { > + return; > + } > + s->registered_yank =3D true; > } > - s->registered_yank =3D true; > > /* be isn't opened until we get a connection */ > *be_opened =3D false > Looks wrong to me, the new chardev will get the same label, and it should still be possible to call the yank functions then. The registration logic needs to be reworked during chardev-change. --=20 Marc-Andr=C3=A9 Lureau --00000000000081529d05bd97bd9c Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hi

On Mon, Mar 15, 2021 at 9:22 PM Li = Zhang <zhlcindy@gmail.com> = wrote:
From: Li = Zhang <li.= zhang@cloud.ionos.com>

When executing the QMP commands "chardev-change" to change the backend device to socket, it will cause a segment fault because
it assumes chr->label as non-NULL in function yank_register_instance. The function qmp_chardev_change calls chardev_new, which label
is NULL when creating a new chardev. The label will be passed to
yank_register_instance which causes a segment fault. The callchain
is as the following:
=C2=A0 =C2=A0 =C2=A0 =C2=A0 chardev_new ->
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qemu_char_open ->
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 cc->open -> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 qmp_chardev_open_so= cket ->
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 yank_= register_instance

Signed-off-by: Li Zhang <li.zhang@cloud.ionos.com>
---
=C2=A0chardev/char-socket.c | 8 +++++---
=C2=A01 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index c8bced76b7..26d5172682 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1421,10 +1421,12 @@ static void qmp_chardev_open_socket(Chardev *chr, =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0qemu_chr_set_feature(chr, QEMU_CHAR_FEATU= RE_FD_PASS);
=C2=A0 =C2=A0 =C2=A0}

-=C2=A0 =C2=A0 if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->la= bel), errp)) {
-=C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
+=C2=A0 =C2=A0 if (chr->label) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (!yank_register_instance(CHARDEV_YANK_INSTA= NCE(chr->label), errp)) {
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return;
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
+=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->registered_yank =3D true;
=C2=A0 =C2=A0 =C2=A0}
-=C2=A0 =C2=A0 s->registered_yank =3D true;

=C2=A0 =C2=A0 =C2=A0/* be isn't opened until we get a connection */
=C2=A0 =C2=A0 =C2=A0*be_opened =3D false

Looks wrong to me, the new chardev will get the same label,= and it should still be possible to call the yank functions then. The regis= tration logic needs to be reworked during chardev-change.
--
Marc-Andr=C3=A9 Lureau<= br>
--00000000000081529d05bd97bd9c--