From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CD6E814E2F2 for ; Thu, 13 Jun 2024 23:50:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718322652; cv=none; b=Ei6ohaewfhB1jI4FjL3oX02y7FrZ9P1tsLL2DLLEpzIPpYXHYPjHx4jubWgvZhB0RgW8gmJfZf2ysftdPCjy9dbO791CIqopjWzXKRAqVquKAQcNDk4Db9vLNyavxU21toi768dCJ9aj2Ny7qbo/hSWGFGsG0PkQKkg9InBeKqo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718322652; c=relaxed/simple; bh=ahRoGy5moLopddVlGfclLvv26he0ry7BCYTrh8QCNpc=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=f1U2+lV+hH59+gAVyuwI+EQ8ismym3eiZPFHbDX5/hdWPlp2FNVM3wr+zWCGivKF8Jc/vHTm+743CL/xQXjaBIqtzBMQ8p885tmS8L3WgNDtW17jE+EeRDuRdGYOX8R7ErJX9i3CWFxsR2t4ZHqplEM9JLhHoiy+W2OgXsbVTMo= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=TUtAKIc6; arc=none smtp.client-ip=209.85.219.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="TUtAKIc6" Received: by mail-yb1-f201.google.com with SMTP id 3f1490d57ef6-dfab38b7f6bso2446735276.0 for ; Thu, 13 Jun 2024 16:50:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1718322650; x=1718927450; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:from:to:cc:subject:date:message-id:reply-to; bh=q/2GD2P1P7CEf4LO1djLMe1zjWvI2Hkpb11N2GxkTnU=; b=TUtAKIc6Q/bwQn/MFPoxJzZfU0sMRrTfbZBgwm1+0ctTj7WUUus7DO9LlNjo8jScjQ fQ2SAwV0XEc4UeY5tsE0NTVJYB7OFEIgKwsYNAPuVbw+kZwczIrqy8sEXJa/DVZeHblc pFbfbmpuh+UlkcFikqnTrrpmHVZCU1u8k5JGR1Ua4F9Thl+1UyzeGSLaZiwe6DUeXAXn T3xCiO5kwqdzuVuTIZIipyEGvKfmvn0P4DPNtP9rSQ6vRXmKkRwvKG4FWD0KmUFxL4lO OZVqbngfJMLqv7gNEYRxciCmiVcLm4W5QCRFKwNEm/5oZEnJH/ri8xlgSIOPBrpgXPRJ 8YBA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718322650; x=1718927450; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=q/2GD2P1P7CEf4LO1djLMe1zjWvI2Hkpb11N2GxkTnU=; b=F490qGUe38VwSr7A6+OlUJBKUJDvheq6ljkGa/UdFautBQBWc5t//Qq2m1Z2ezuA3B 8KOY/zvX1pOR09iMunMeYkzIQKotY1F+PvCdrnl7nWqItKM3Z4M8Zkv7q6h1gKS0Floc fwDAhUAb706cMaL/FiVaYCcMqo2TAGxR27FElOTohJpHgqEJ3/1Zv5Gs3omYoUB4KqVQ BEL3XQI9rGeTbDEsN75wNJw2ZF3RTya++Wmln9M5c9qxKKi9FBjY7qD6YTjW6jWmJqEF 75DKVSEUYPK9EHJOWQq++s7LvLkIn5XjSJ7j7lYkzBUB8tHCquslCaJM3hBjiSsI7ZMP FdvA== X-Forwarded-Encrypted: i=1; AJvYcCU8Hdp2DEMhuUd1j7d+I0rjZkOeglkzjUMrXM5qspuJHdODF4KjJeZIt1GvLIznjlfAmAHPC7mZsVjFKf+uOPah359vPawSrOoGBVo6 X-Gm-Message-State: AOJu0Yyk9eMBpiq52hnmT03Gpyujldh+AEfnz9yMaEfY+p69kbu/EHRw om72xc4MhLoK+Fe5EbiATvawoUlkJlvwYBCb20NORPxMwgMjGfAaxIBUVFwfetG4Cn7eMreTzcP 4pw== X-Google-Smtp-Source: AGHT+IGdwT4otzma8gQNBf9ykyP8JCZXHjwasEwBqEvekHLX0RYn2KS31UOPwPFM3LcoNrjyVrWpomgD6Og= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a05:6902:2e13:b0:dfb:bf0:59cd with SMTP id 3f1490d57ef6-dff153b2e10mr184365276.7.1718322649837; Thu, 13 Jun 2024 16:50:49 -0700 (PDT) Date: Thu, 13 Jun 2024 16:50:48 -0700 In-Reply-To: <02051e0a-09d8-49a2-917f-7c2f278a1ba1@moroto.mountain> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <02051e0a-09d8-49a2-917f-7c2f278a1ba1@moroto.mountain> Message-ID: Subject: Re: [PATCH] KVM: fix an error code in kvm_create_vm() From: Sean Christopherson To: Dan Carpenter Cc: Yi Wang , Paolo Bonzini , Christian Borntraeger , kvm@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-janitors@vger.kernel.org Content-Type: text/plain; charset="us-ascii" On Thu, Jun 13, 2024, Dan Carpenter wrote: > This error path used to return -ENOMEM from the where r is initialized > at the top of the function. But a new "r = kvm_init_irq_routing(kvm);" > was introduced in the middle of the function so now the error code is > not set and it eventually leads to a NULL dereference. Set the error > code back to -ENOMEM. > > Fixes: fbe4a7e881d4 ("KVM: Setup empty IRQ routing when creating a VM") > Signed-off-by: Dan Carpenter > --- > virt/kvm/kvm_main.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 07ec9b67a202..ea7e32d722c9 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -1212,8 +1212,10 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) > for (i = 0; i < KVM_NR_BUSES; i++) { > rcu_assign_pointer(kvm->buses[i], > kzalloc(sizeof(struct kvm_io_bus), GFP_KERNEL_ACCOUNT)); > - if (!kvm->buses[i]) > + if (!kvm->buses[i]) { > + r = -ENOMEM; > goto out_err_no_arch_destroy_vm; > + } > } Drat. Any objection to tweaking this slightly to guard against similar bugs in the future? If not, I'll apply+push the below. Thanks! -- From: Dan Carpenter Date: Thu, 13 Jun 2024 17:33:16 +0300 Subject: [PATCH] KVM: fix an error code in kvm_create_vm() This error path used to return -ENOMEM from the where r is initialized at the top of the function. But a new "r = kvm_init_irq_routing(kvm);" was introduced in the middle of the function so now the error code is not set and it eventually leads to a NULL dereference. Set the error code back to -ENOMEM. Opportunistically tweak the logic to pre-set "r = -ENOMEM" immediately before the flows that can fail due to memory allocation failure to make it less likely that the bug recurs in the future. Fixes: fbe4a7e881d4 ("KVM: Setup empty IRQ routing when creating a VM") Signed-off-by: Dan Carpenter Link: https://lore.kernel.org/r/02051e0a-09d8-49a2-917f-7c2f278a1ba1@moroto.mountain [sean: tweak all of the "r = -ENOMEM" sites] Signed-off-by: Sean Christopherson --- virt/kvm/kvm_main.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index b60186b9c1d3..436ca41f61e5 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1143,8 +1143,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) { struct kvm *kvm = kvm_arch_alloc_vm(); struct kvm_memslots *slots; - int r = -ENOMEM; - int i, j; + int r, i, j; if (!kvm) return ERR_PTR(-ENOMEM); @@ -1181,6 +1180,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) snprintf(kvm->stats_id, sizeof(kvm->stats_id), "kvm-%d", task_pid_nr(current)); + r = -ENOMEM; if (init_srcu_struct(&kvm->srcu)) goto out_err_no_srcu; if (init_srcu_struct(&kvm->irq_srcu)) @@ -1209,6 +1209,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname) rcu_assign_pointer(kvm->memslots[i], &kvm->__memslots[i][0]); } + r = -ENOMEM; for (i = 0; i < KVM_NR_BUSES; i++) { rcu_assign_pointer(kvm->buses[i], kzalloc(sizeof(struct kvm_io_bus), GFP_KERNEL_ACCOUNT)); base-commit: 3dee3b187499b317a6587e2b8e9bf3d5050e5288 --