Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756527AbdCXJfC (ORCPT ); Fri, 24 Mar 2017 05:35:02 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:34569 "EHLO mx0a-001b2d01.pphosted.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752344AbdCXJex (ORCPT ); Fri, 24 Mar 2017 05:34:53 -0400 Date: Fri, 24 Mar 2017 10:33:55 +0100 From: Cornelia Huck To: David Hildenbrand Cc: Dmitry Vyukov , Marcelo Tosatti , KVM list , Paolo Bonzini , Radim =?UTF-8?B?S3LEjW3DocWZ?= , stable , LKML Subject: Re: [PATCH v2] KVM: kvm_io_bus_unregister_dev() should never fail In-Reply-To: <0dd97243-db9b-4d22-970e-489d0f491851@redhat.com> References: <20170323172419.21435-1-david@redhat.com> <20170323204247.GC27861@amt.cnet> <0dd97243-db9b-4d22-970e-489d0f491851@redhat.com> Organization: IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz =?UTF-8?B?R2VzY2jDpGZ0c2bDvGhydW5nOg==?= Dirk Wittkopp Sitz der Gesellschaft: =?UTF-8?B?QsO2Ymxpbmdlbg==?= Registergericht: Amtsgericht Stuttgart, HRB 243294 X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.23; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 x-cbid: 17032409-0016-0000-0000-00000463694A X-IBM-AV-DETECTION: SAVI=unused REMOTE=unused XFE=unused x-cbparentid: 17032409-0017-0000-0000-000026F2829D Message-Id: <20170324103355.73bb95ec.cornelia.huck@de.ibm.com> X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-03-24_08:,, signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1702020001 definitions=main-1703240084 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1274 Lines: 33 On Fri, 24 Mar 2017 09:55:15 +0100 David Hildenbrand wrote: > > >>> - return r; > >>> + if (i == bus->dev_count) > >>> + return; > >>> > >>> new_bus = kmalloc(sizeof(*bus) + ((bus->dev_count - 1) * > >>> sizeof(struct kvm_io_range)), GFP_KERNEL); > >>> - if (!new_bus) > >>> - return -ENOMEM; > >>> + if (!new_bus) { > >>> + pr_err("kvm: failed to shrink bus, removing it completely\n"); > >>> + goto broken; > >> > >> The guest will fail in mysterious ways, if you do this (and > >> io_bus_unregister_dev can be called during runtime): in-kernel device > >> accesses will fail with unknown behaviour in the guest. > > Actually, the next access to the BUS should result in -ENOMEM. And the > error message should be enough to then figure out what went wrong. Hopefully, an admin will look at the logs :) But yes, the patch should have caught all issues in the host, and the guest will basically be presented with broken "hardware". > However, to hit this scenario at all feels very unlikely. So I would > like to avoid advanced allocation schemes. Agreed, spending too much time on complex recovery scenarios is overkill for this unlikely case.