From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Mimi Zohar" <zohar@linux.ibm.com>,
"Dan Williams" <dan.j.williams@intel.com>,
"Verma, Vishal L" <vishal.l.verma@intel.com>,
"paul@paul-moore.com" <paul@paul-moore.com>,
"dhowells@redhat.com" <dhowells@redhat.com>,
"yaelt@google.com" <yaelt@google.com>,
"serge@hallyn.com" <serge@hallyn.com>,
"nichen@iscas.ac.cn" <nichen@iscas.ac.cn>,
"sumit.garg@linaro.org" <sumit.garg@linaro.org>,
"jmorris@namei.org" <jmorris@namei.org>
Cc: "Jiang, Dave" <dave.jiang@intel.com>,
"linux-integrity@vger.kernel.org"
<linux-integrity@vger.kernel.org>,
"linux-cxl@vger.kernel.org" <linux-cxl@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
"linux-security-module@vger.kernel.org"
<linux-security-module@vger.kernel.org>,
"nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>
Subject: Re: [PATCH] KEYS: encrypted: Add check for strsep
Date: Mon, 12 Feb 2024 05:11:57 +0000 [thread overview]
Message-ID: <CZ2UCEZ1VT96.2QZE7X8CS8EJ2@seitikki> (raw)
In-Reply-To: <d0ccd2f19ed1adccc8f3dfe677c30bc44feb3d36.camel@linux.ibm.com>
On Fri Feb 2, 2024 at 12:05 AM UTC, Mimi Zohar wrote:
> On Thu, 2024-02-01 at 23:43 +0200, Jarkko Sakkinen wrote:
> > On Tue Jan 30, 2024 at 8:25 PM EET, Dan Williams wrote:
> > > Jarkko Sakkinen wrote:
> > > > On Tue Jan 30, 2024 at 7:22 PM EET, Jarkko Sakkinen wrote:
> > > > > On Wed Jan 24, 2024 at 11:10 PM EET, Verma, Vishal L wrote:
> > > > > > On Wed, 2024-01-24 at 15:40 -0500, Mimi Zohar wrote:
> > > > > > > On Wed, 2024-01-24 at 20:10 +0000, Verma, Vishal L wrote:
> > > > > > > > Ah, thanks for confirming! Would you like me to send a
> > > > > > > > revert patch or
> > > > > > > > will you do it?
> > > > > > >
> > > > > > > Revert "KEYS: encrypted: Add check for strsep"
> > > > > > >
> > > > > > > This reverts commit
> > > > > > > b4af096b5df5dd131ab796c79cedc7069d8f4882.
> > > > > > >
> > > > > > > New encrypted keys are created either from kernel-generated
> > > > > > > random
> > > > > > > numbers or user-provided decrypted data. Revert the change
> > > > > > > requiring
> > > > > > > user-provided decrypted data.
> > > > > > >
> > > > > > >
> > > > > > > Can I add your Reported-by?
> > > > > >
> > > > > > Yes that works, Thank you.
> > > > >
> > > > > This went totally wrong IMHO.
> > > > >
> > > > > Priority should be to locate and fix the bug not revert useful
> > > > > stuff
> > > > > when a bug is found that has limited scope.
> > > >
> > > > By guidelines here the commit is also a bug fix and reverting
> > > > such commit means seeding a bug to the mainline. Also the klog
> > > > message alone is a bug fix here. So also by book it really has
> > > > to come back as it was already commit because we cannot
> > > > knowingly mount bugs to the mainline, right?
> > >
> > > No, the commit broke userspace. The rule is do not cause
> > > regressions
> > > even if userspace is abusing the ABI in an undesirable way. Even
> > > the
> > > new pr_info() is a log spamming behavior change, a pr_debug() might
> > > be
> > > suitable, but otherwise a logic change here needs a clear
> > > description
> > > about what is broken about the old userspace behavior and why the
> > > kernel
> > > can not possibly safely handle it.
> >
> > The rationale literally gives empirical proof that the log message
> > is useful by measure. It would be useless if log level is decreased
> > to debug, as then sysadmin's won't take notice. I don't really know
> > what is the definition of "spam" here but at least for me actually
> > useful log message are not in that category.
> >
> > Issue was legit but git revert is objectively an incorrect way to
> > address the bug.
>
> No, I made a mistake in upstreaming the patch in the first place. It
> broke the original "encrypted" keys usage. Reverting it was the
> correct solution.
>
> Mimi
The way I see it the semantic change caused the bug because it was not
backwards compatible. That does not make the log message less useful.
BR, Jarkko
next prev parent reply other threads:[~2024-02-12 5:12 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-08 7:36 [PATCH] KEYS: encrypted: Add check for strsep Chen Ni
2024-01-24 18:21 ` Verma, Vishal L
2024-01-24 19:15 ` Mimi Zohar
2024-01-24 20:10 ` Verma, Vishal L
2024-01-24 20:40 ` Mimi Zohar
2024-01-24 21:10 ` Verma, Vishal L
2024-01-30 17:22 ` Jarkko Sakkinen
2024-01-30 17:30 ` Jarkko Sakkinen
2024-01-30 18:25 ` Dan Williams
2024-02-01 21:43 ` Jarkko Sakkinen
2024-02-02 0:05 ` Mimi Zohar
2024-02-12 5:11 ` Jarkko Sakkinen [this message]
2024-01-30 17:19 ` Jarkko Sakkinen
2024-01-30 17:20 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CZ2UCEZ1VT96.2QZE7X8CS8EJ2@seitikki \
--to=jarkko@kernel.org \
--cc=dan.j.williams@intel.com \
--cc=dave.jiang@intel.com \
--cc=dhowells@redhat.com \
--cc=jmorris@namei.org \
--cc=keyrings@vger.kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=nichen@iscas.ac.cn \
--cc=nvdimm@lists.linux.dev \
--cc=paul@paul-moore.com \
--cc=serge@hallyn.com \
--cc=sumit.garg@linaro.org \
--cc=vishal.l.verma@intel.com \
--cc=yaelt@google.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).