From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f52.google.com (mail-wr1-f52.google.com [209.85.221.52]) (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 99F2F33D501 for ; Thu, 4 Jun 2026 10:18:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.52 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780568310; cv=none; b=J51vwxW4N4/00YvvVLH0hiPlVqdQe3ue5hVBkgWnXjiue5pTOifABSr5oIJgHKjqgNgCpVHnpUIEBy2MBkvAKoBln1NoiKUT8YcSJdYEGNxe+I8mDMr9A8g5Gg12zfEbHCyIPo+WzrXPp8vlti4WmwB5RUg/FX/kEqqeIktb90o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780568310; c=relaxed/simple; bh=jghp1SrtVtQziL5WscWqI2sNGta4aCH2vSJxCnUnH28=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SXESdE6nMFhcl4DAQyvpCf4IVLkuA3eJycyMeuk3avNo5PUKWdeWhJq/oSHF65Nzk8tAHSAwx8mmd4cBiT+1zg1z9rlVwFzcfSik3VpkC5dMmZIkwQjL4srWrB048tEurcQ6e6bNh99bgzb24JsVHXjX6tLSS1uHSwVBTdBeM/0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=dE9EWZpO; arc=none smtp.client-ip=209.85.221.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="dE9EWZpO" Received: by mail-wr1-f52.google.com with SMTP id ffacd0b85a97d-45ef1629ff4so380656f8f.0 for ; Thu, 04 Jun 2026 03:18:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1780568307; x=1781173107; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=P2+oaEcIfz7GmsSjwEdMlieUef5BxbWRvsgFu7sckH0=; b=dE9EWZpOo0qGtUxKmD5TBxLbjl0aNjTgagk+E+fih59xJWSUHOidE6TQqKXEstEL1q cuRaesTVRs2jSr0hp8uiEC6Ib2COt8qck3+QYjvNzk/YZ5ZMMbkkPcgcFkxc7zGtXtSG 27D+zM4p5CnaS6/9MAbJLzQZq7CGeXwDL0t2eWyZBRzPcSn6cJ/MmNUDZ8O5H767nusP hoGKoM1pC00iQOGDCv+xTy8pGoBlf8cxnl/oPYinvD7OoBAasw6KtJPKQWtfPf4MG/9O 6Gyo/E0OvSdiLB6IUKFZrj63u0Fu5oxn/ZhcZCWvO+mZ1papUNdUS832ICWU6eaIEyLl pjLw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1780568307; x=1781173107; h=in-reply-to:content-disposition: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; bh=P2+oaEcIfz7GmsSjwEdMlieUef5BxbWRvsgFu7sckH0=; b=qMjY6JO/t9mXLtMVtdWPqZsyrSA/4IbDp8HFZDOVfhzDJRBejF+T1wm97/3BHk8oCI Mn9plTkM5Q648JI4wbiPqVuWRO1jtXzahg2xKljoqOvauW7FJ07dcRpZ7i8EjLwp/E7/ H+zF5d2kZeKxgi8wIjfjJumovorzWFA7d86/vSBuxbQHhgjHRvhY/MDPlr4Khj4FRsPS 0LlXoSUMnX8SD5sh6HlWcut/V72OVxELstevK/+QLGSoOm3LFPZ72E2+JTDNRkWzTzDE Z2WQVCsKL/yrrXKvZHZvy6sDK3ogUKk8BN3wNpafldlNEUlU0b+gCkxGamGP33ba2cTB GaJg== X-Forwarded-Encrypted: i=1; AFNElJ85eLJk4/0RiqcxS85VRk243obA0DByvRbaJgsKs8fGzCtERW3YqJhqgW4j2Epe4QLU0Tf82vAtlbq9tXo=@vger.kernel.org X-Gm-Message-State: AOJu0YzRapPnKhCdGlth8F+sdyZueZl9oA3OTDstmkwgz8QgSTqAoqdW B2UsB+QV6CrithzgtWlgFSk/NTz89R9avYMQC059AnAoysiSmStQgQj38Nk5PYoQwj8= X-Gm-Gg: Acq92OGCUq98AfrGp8Hi3+Zmu7eOQJOyfvWjc3eXXen5UJ6Fq+Zw1igcwJaSEpBBT0i PRCSFU7PjplxyFAZsOL72o2KJqh4sZK5NjYl+kPJ7nuSsVERsmSfllxqtpfq6rjtRalpd5309EI AazJwVWZijOXX69t3YnuYKzkF0Y8kqoHE/n5fGZL1VoCnD2P/kwi2xgWQS96zorYjiQZ2KCpZiW SE5fAmPU8MK7dVgUgO8rBcQUL8mxCS84Y9loX3xZ1Z5cEr8cjOziYyff8O4LC2uKvfGTkvr2+IE hk29nhUp6Lv0Hze1OYOj0JN7nQoAcddavXXOqBHFGsqZBDKFey6Ik0Pj9eFmt3b8MuYCNdTRplM /kYRlC6LQAFT8CChfpeoU4ZG1abLl/MN+7iaJnVtVpHaOtTTp65k6tzk78ZhMqsJZvULUQ82gRB gIuAVvpBmpwFbtaXS+C3u/Mv93xXAjPA8WmbnK X-Received: by 2002:a5d:64e4:0:b0:460:1e5a:2267 with SMTP id ffacd0b85a97d-460217cdeaamr11841706f8f.17.1780568306997; Thu, 04 Jun 2026 03:18:26 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-4601f2dc412sm15595036f8f.4.2026.06.04.03.18.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 04 Jun 2026 03:18:26 -0700 (PDT) Date: Thu, 4 Jun 2026 12:18:24 +0200 From: Petr Mladek To: Naveen Kumar Chaudhary Cc: Steven Rostedt , John Ogness , Sergey Senozhatsky , linux-kernel@vger.kernel.org Subject: Re: [PATCH] printk: fix out-of-bounds access in try_enable_preferred_console() Message-ID: References: <7sq4tr2nmlz32tvkf6vpsghv6exvqfghsrlvywjcqihzsqqbf7@bspclmti5xg4> 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=us-ascii Content-Disposition: inline In-Reply-To: <7sq4tr2nmlz32tvkf6vpsghv6exvqfghsrlvywjcqihzsqqbf7@bspclmti5xg4> (Once again with corrected LKML address. I am sorry for noice.) Adding other printk subsystem reviewers into Cc. There is get_maintainer.pl script for this purpose. For example: $> ./scripts/get_maintainer.pl kernel/printk/printk.c Petr Mladek (maintainer:PRINTK) Steven Rostedt (reviewer:PRINTK) John Ogness (reviewer:PRINTK) Sergey Senozhatsky (reviewer:PRINTK) linux-kernel@vger.kernel.org (open list) On Sat 2026-05-30 10:18:24, Naveen Kumar Chaudhary wrote: > When all MAX_CMDLINECONSOLES (8) slots in console_cmdline[] are occupied > and none match the newly registered console, the for loop exits with > i == MAX_CMDLINECONSOLES and c pointing past the end of the array. The > subsequent access to c->user_specified is then an out-of-bounds read. Great catch! > This can occur when a self-enabling console (one with CON_ENABLED already > set), such as netconsole or pstore, calls register_console() on a system > where the console_cmdline[] array has been filled by a combination of > command-line console= parameters, ACPI SPCR, device tree stdout-path, > and/or arch-specific add_preferred_console() calls. > > Add a bounds check to ensure c is only dereferenced when the loop exited > due to finding an empty slot (i.e., c still points within the array). > Also add parentheses around the bitwise-AND to silence compiler warnings > about its use in a boolean context. But the fix is is not correct, see below. > --- a/kernel/printk/printk.c > +++ b/kernel/printk/printk.c > @@ -3938,7 +3938,8 @@ static int try_enable_preferred_console(struct console *newcon, > * without matching. Accept the pre-enabled consoles only when match() > * and setup() had a chance to be called. > */ > - if (newcon->flags & CON_ENABLED && c->user_specified == user_specified) > + if (i < MAX_CMDLINECONSOLES && (newcon->flags & CON_ENABLED) && > + c->user_specified == user_specified) This would prevent the out-of-bound access to c->user_specified. But the check of c->user_specified does _not_ make sense in the first place. Background: ----------- The idea was that we would allow to match a preferred console and run newcon->setup(). By other words, this code should be called in register_console() after /* See if this console matches one we selected on the command line */ err = try_enable_preferred_console(newcon, true); /* If not, try to match against the platform default(s) */ if (err == -ENOENT) err = try_enable_preferred_console(newcon, false); , when try_enable_preferred_console() did not return a real error. The real error is an error from newcon->setup(). Note that -ENOENT is not meant as a real error here. Solution: --------- IMHO, the right approach is to move this code out of try_enable_preferred_console(). Like it is done in my clean up of the registration code. I have just sent v3 earlier today, see https://lore.kernel.org/all/20260602085312.228251-10-pmladek@suse.com/ Impact: ------- Now, the question is how serious this bug is. Do we need to fix it now? Or is it enough to wait for the clean up? IMHO, the only danger is that the kernel might trigger an invalid access and panic(). But is this possible? console_cmdline[] is stored in the initialized data segment. I guess that there are always another "valid" static data right after it. So, it should "never" trigger an invalid access. Reading invalid data should not cause big problems. In the worst case, the console will get enabled in try_enable_preferred_console(newcon, true) instead of try_enable_preferred_console(newcon, true) and newcon->setup() won't get caller. But I think that pre-enabled console drivers should never rely on calling this, anyway. Finally, it is hard to imagine that anyone would use all 8 slots for preferred console entries. So, this should be almost impossible to hit in practice. That said, I think about backporting the 10th patch from the above mentioned clean up and fixing this ASAP. Better be safe than sorry... Best Regards, Petr > return 0; > > return -ENOENT; > -- > 2.43.0