From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) (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 EDE83413D8B for ; Fri, 10 Jul 2026 11:13:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783682031; cv=none; b=FfMI+maiiuIRbjRnPNlGtM2/RYrc696BZ5AC1hOmuqBZZ4HwiWQcZpTBl3iTq+lZn2ugmXM+hMPt/2Qt6jElC57Kps2vLCePUXCSg3mDkzYLTq7nerMI6IwsdhrIy3mvsV+oQrbMuHO4ei2MTV9G+tkqHd/zw68QhkPWEll5rDE= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783682031; c=relaxed/simple; bh=c4OVHU38z7vfkatYPQKtn8vnc7akYZidlGLsN81aP0Y=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=u+vXQDKO7HH5L+DUcD4XMNnwjiMqi402Fk2mTQdUZNfhzdJAGJhGgiz4HIlz7XkW7X20n11/+AVhNGIrIvofB4JfpzP0QkqOxQYOUAJR8hBfaL79rlGKBazodaCocTCyGl499cpHkKzjYNFcmk+r8DFlz8js0H00v5LhaXRMcWA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linbit.com; spf=pass smtp.mailfrom=linbit.com; dkim=pass (2048-bit key) header.d=linbit-com.20251104.gappssmtp.com header.i=@linbit-com.20251104.gappssmtp.com header.b=Gu3tOBcG; arc=none smtp.client-ip=209.85.221.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linbit.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linbit.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linbit-com.20251104.gappssmtp.com header.i=@linbit-com.20251104.gappssmtp.com header.b="Gu3tOBcG" Received: by mail-wr1-f50.google.com with SMTP id ffacd0b85a97d-4728c12ba97so376177f8f.0 for ; Fri, 10 Jul 2026 04:13:48 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linbit-com.20251104.gappssmtp.com; s=20251104; t=1783682027; x=1784286827; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:mail-followup-to:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to :content-type; bh=ODgufXz1PhelMlB2W5pp8vf7ivTLwxqDccyqSe9smQ8=; b=Gu3tOBcGf3T0pnU9ybwJ1OIj0sxHVMkkaVxMCpYU1p+VQfXrRcf8t/tZq4Nl9iRgrb jedOKWmhfFdqNIBXQuF27OcoYXMobk5QXFhM/CeFArNDEzILbQscmFisasjsQsFVFmT6 n70WRs3rocq8pj9uCxgEYoqUVecNlXWK5WWyrBfZ7lkjY4imeN5GobWtbADjQSazOER3 zUJs1w1n/cUWM+k/n9t7cAxyAF708wvv8Xnfq89Gg4VZfqLCP1u+Rx3xLeuUA/C7xYCm zzb/QmgHnpBVTyUyOFFkaIipLq1JCIf3sFKPvgz9iaSMXmUSp5WOzTOHyJOLhFw5Fixc BOkw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1783682027; x=1784286827; h=in-reply-to:content-transfer-encoding:content-disposition :content-type:mime-version:references:mail-followup-to: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=ODgufXz1PhelMlB2W5pp8vf7ivTLwxqDccyqSe9smQ8=; b=AJZsTDUI17HxNhZEc5Grp6qxhjS8K1APtFSmX8hULWipvklX3MIskehHY28RHsrUFb VgpaQttMhH6nF6/FRzn7U/3GrC1xvkPPHlQk77NrsJSXLXgYrljdOMoRVd5xNF22A0NY 0xUXfbbkBroGxMOQatCtKEr/4wxGX0Pr1e+7pAuhjYSKCyyC7hXRaA+FovpxWzDY3snn L/cLXWNZZnRjTbU193q3jr7t80XT3NeU94A8QPjPmvp7Hp7co8vWT4dQ584dAkA7LFOO xJaOxwgEj5EcK8KiCgtDeZLDVUuTBG/87DWHBY7LT67au+k8JYmFItSqYDLsWgeYBlxZ 8w0g== X-Forwarded-Encrypted: i=1; AHgh+Rpo5LHzju1ENt8RxLzJnihaEOLJsJjqZCMai4px6aouyjje0OIxW/zcLTM8Go8EgwmdRiq2Vkd/1ZrguY8=@vger.kernel.org X-Gm-Message-State: AOJu0Yz9yiQtti0n8f/AHzS+8+G7PM1z52yyBeKuew55bPe/7DFZO1bI lBXwNoOKWCCFD0Ie97AXdB8QEzNCQryCdYT38D/F63IPYlA5xcljcB8ww1VwKW6UDSc= X-Gm-Gg: AfdE7cl/6hSlkMnO+iUiov8jIbsL7sL0ehYPW5pW9x8m3wSsilW98mDGKh4FWW1RFh6 ndgAVbNKbGw3/3jVAIA5v/Twpx0QDKOrsazip2fYE8IzHx62971G4vrXVSbIqQbHJbopxlO9sSm Q+fhrQnPRMCM4tOsZ1AVaMTW9Fuwu5KajUqmh18sC8x8PO0+ihsycIGqUOtf4Qi3/ZxrwXdF9Rt FTueAPLAASyejs4ydIZ8R55W3fipT+V30pSJ3xAH2Z7sGQSGN84d4UmFYYRQtQiItKruyEnqezO P9hvbKWg4Y3Gqc7drkuvCDJLwhbXPDZtQor2hZaAqZPH8gdOYZGpQ1T0+ZrT4hrCmIqcfeIIAbk /p5y0mOY/2xg8jGoDMXpwkzWA8BIpGEVrFIi/DwrarDd3S/gW+VR0fzzq4xqAtelM8dfxeQWXMT Jzu5g1vRDrNFGDCI1hc2yjqVOctUpm2M87XHKFBxwN5fa05lYylsludYwn X-Received: by 2002:a05:6000:2482:b0:477:6874:5415 with SMTP id ffacd0b85a97d-47df074605amr11973058f8f.26.1783682027154; Fri, 10 Jul 2026 04:13:47 -0700 (PDT) Received: from localhost (h082218129081.host.wavenet.at. [82.218.129.81]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-47a9e4d6e4csm56399132f8f.10.2026.07.10.04.13.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 10 Jul 2026 04:13:46 -0700 (PDT) Date: Fri, 10 Jul 2026 13:13:45 +0200 From: Christoph =?utf-8?Q?B=C3=B6hmwalder?= To: =?utf-8?B?0JLQsNGC0L7RgNC+0L/QuNC9INCQ0L3QtNGA0LXQuQ==?= Cc: Philipp Reisner , Lars Ellenberg , Jens Axboe , Andreas Gruenbacher , "drbd-dev@lists.linbit.com" , "linux-block@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "lvc-project@linuxtesting.org" , "stable@vger.kernel.org" Subject: Re: [PATCH] drbd: Fix potential NULL pointer dereference in _drbd_set_state() Message-ID: Mail-Followup-To: =?utf-8?B?0JLQsNGC0L7RgNC+0L/QuNC9INCQ0L3QtNGA0LXQuQ==?= , Philipp Reisner , Lars Ellenberg , Jens Axboe , Andreas Gruenbacher , "drbd-dev@lists.linbit.com" , "linux-block@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "lvc-project@linuxtesting.org" , "stable@vger.kernel.org" References: <20260625050016.12004-1-a.vatoropin@crpt.ru> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20260625050016.12004-1-a.vatoropin@crpt.ru> Thanks for your patch. On Thu, Jun 25, 2026 at 05:03:06AM +0000, Ваторопин Андрей wrote: >From: Andrey Vatoropin > >The connection pointer receives a value in the _drbd_set_state() >function, including through a call to the first_peer_device() function. >This function returns a pointer to a list element. If the list is empty, it >returns a NULL pointer, which is later assigned to the connection >pointer. Subsequently, this pointer will be dereferenced. Can the list actually be empty at this point? The peer_device is linked into the list in drbd_create_device(), before add_disk() and before the device is inserted into connection->peer_devices, so no state change can reach the device earlier. It is only unlinked again in drbd_destroy_device(), after the last kref to the device is gone. The connection itself is created together with the resource in drbd_adm_new_resource() and lives until the resource is destroyed. So for any device this function can be called on, first_peer_device() returns a valid peer_device. > >Add a NULL check for the connection pointer to avoid dereferencing an >invalid pointer. > >Found by Linux Verification Center (linuxtesting.org) with SVACE. > >Fixes: a6b32bc3cebd ("drbd: Introduce "peer_device" object between "device" and "connection"") >Cc: stable@vger.kernel.org >Signed-off-by: Andrey Vatoropin >--- > drivers/block/drbd/drbd_state.c | 5 +++++ > 1 file changed, 5 insertions(+) > >diff --git a/drivers/block/drbd/drbd_state.c b/drivers/block/drbd/drbd_state.c >index adcba7f1d8ea..ea982d48017e 100644 >--- a/drivers/block/drbd/drbd_state.c >+++ b/drivers/block/drbd/drbd_state.c >@@ -1281,6 +1281,11 @@ _drbd_set_state(struct drbd_device *device, union drbd_state ns, > if (rv < SS_SUCCESS) > return rv; > >+ if (!connection) { >+ drbd_err(device, "No connection to peer, aborting!\n"); >+ return SS_ALREADY_STANDALONE; >+ } >+ Also, even if the condition could happen, its handling here would be wrong. Since this check happens before handling hard state changes, those could potentially be skipped, which is not allowed. For example, after a local I/O error (drbd_chk_io_error), if this condition would trigger, the detach state change would be silently skipped. So in that circumstance, this patch would be actively harmful. Also, SS_ALREADY_STANDALONE would map to the error message "Can not disconnect a StandAlone device", which does not make any sense in this context. > if (!(flags & CS_HARD)) { > /* pre-state-change checks ; only look at ns */ > /* See drbd_state_sw_errors in drbd_strings.c */ >-- >2.43.0 In summary, unless I missed something major: NAK. Thanks, Christoph