* Re: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Mimi Zohar @ 2021-07-30 12:39 UTC (permalink / raw)
To: Roberto Sassu, gregkh@linuxfoundation.org,
mchehab+huawei@kernel.org
Cc: linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ef7c85dcb096479e95c8c60ccda4d700@huawei.com>
Hi Roberto,
On Fri, 2021-07-30 at 07:16 +0000, Roberto Sassu wrote:
> > From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> > Sent: Thursday, July 29, 2021 11:21 PM
> >
> > On Mon, 2021-07-26 at 18:36 +0200, Roberto Sassu wrote:
> > > /*
> > > + * digest_list_read: read and parse the digest list from the path
> > > + */
> > > +static ssize_t digest_list_read(char *path, enum ops op)
> > > +{
> > > + void *data = NULL;
> > > + char *datap;
> > > + size_t size;
> > > + u8 actions = 0;
> > > + struct file *file;
> > > + char event_name[NAME_MAX + 9 + 1];
> > > + u8 digest[IMA_MAX_DIGEST_SIZE] = { 0 };
> > > + enum hash_algo algo;
> > > + int rc, pathlen = strlen(path);
> > > +
> > > + /* Remove \n. */
> > > + datap = path;
> > > + strsep(&datap, "\n");
> > > +
> > > + file = filp_open(path, O_RDONLY, 0);
> > > + if (IS_ERR(file)) {
> > > + pr_err("unable to open file: %s (%ld)", path, PTR_ERR(file));
> > > + return PTR_ERR(file);
> > > + }
> > > +
> > > + rc = kernel_read_file(file, 0, &data, INT_MAX, NULL,
> > > + READING_DIGEST_LIST);
> > > + if (rc < 0) {
> > > + pr_err("unable to read file: %s (%d)", path, rc);
> > > + goto out;
> > > + }
> > > +
> > > + size = rc;
> > > +
> > > + snprintf(event_name, sizeof(event_name), "%s_file_%s",
> > > + op == DIGEST_LIST_ADD ? "add" : "del",
> > > + file_dentry(file)->d_name.name);
> > > +
> > > + rc = ima_measure_critical_data("diglim", event_name, data, size, false,
> > > + digest, sizeof(digest));
> > > + if (rc < 0 && rc != -EEXIST)
> > > + goto out_vfree;
> >
> > The digest lists could easily be measured while reading the digest list
> > file above in kernel_read_file(). What makes it "critical-data"? In
> > the SELinux case, the in memory SELinux policy is being measured and
> > re-measured to make sure it hasn't been modified. Is the digest list
> > file data being measured more than once?
>
> Hi Mimi
>
> yes, the digest lists can be measured with kernel_read_file().
> I didn't send the change yet, but I added a DIGEST_LIST_CHECK
> hook mapped to READING_DIGEST_LIST, so that digest lists
> can be easily measured or appraised.
>
> The point was that the digest of the digest list must be always
> calculated, as it is added to the hash table. Instead of duplicating
> the code, I preferred to use ima_measure_critical_data().
>
> The advantage is also that, if the use case is to just measure
> digest lists, ima_measure_critical_data() could do both at the
> same time.
>
> Digest lists can be seen as "critical data" in the sense that
> they can affect the security decision on whether to grant
> access to a file or not, assuming that an appropriate rule is
> added in the IMA policy.
Of course the integrity of files containing the digest lists is
important, but that doesn't make them "critical data". If the
integrity of these files is important, then the digest lists not only
need to be measured, but they need to be signed and the resulting
signature verified. Without signature verification, there is no basis
on which to trust the digest lists data.
Adding the kernel_read_file() "READING_DIGEST_LIST" support in IMA does
not seem to be optional. IMA would then be calculating the digest list
file hash twice, once in kernel_read_file() and then, again, in
ima_measure_critical_data().
>
> > I understand that with your changes to ima_measure_critical_data(),
> > which are now in next-integrity-testing branch, allow IMA to calculate
> > the file data hash.
>
> Yes, correct. But actually there is another useful use case.
> If digest lists are not in the format supported by the kernel,
> the user space parser has to convert them before uploading
> them to the kernel.
>
> ima_measure_critical_data() would in this case measure
> the converted digest list (it is written directly, without
> sending the file path). It is easier to attest the result,
> instead of determining whether the user space parser
> produced the expected result (by checking the files it
> read).
The application to properly convert the digest list file data into the
appropriate format would need to be trusted. I'm concerned that not
requiring the converted data to be signed and the signature verified is
introducing a new integrity gap. Perhaps between an LSM policy,
limiting which files may be read by the application, and an IMA policy,
requiring all files read by this application to be measured and the
signature verified, this integrity gap could be averted.
"critical data", in this context, should probably be used for verifying
the in memory file digests and other state information haven't been
compromised.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Mickaël Salaün @ 2021-07-30 12:15 UTC (permalink / raw)
To: Alejandro Colomar (man-pages)
Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
Vincent Dagonneau, landlock, linux-kernel, linux-man,
linux-security-module, Mickaël Salaün, Michael Kerrisk
In-Reply-To: <3f1b943b-2477-2c4e-c835-d6616888176c@gmail.com>
On 29/07/2021 16:56, Alejandro Colomar (man-pages) wrote:
> Hi Mickaël,
>
> On 7/12/21 5:57 PM, Mickaël Salaün wrote:
>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>
>> From the user point of view, Landlock is a set of system calls enabling
>> to build and enforce a set of access-control rules. A ruleset can be
>> created with landlock_create_ruleset(2), populated with
>> landlock_add_rule(2) and enforced with landlock_restrict_self(2). This
>> man page gives an overview of the whole mechanism. Details of these
>> system calls are documented in their respective man pages.
>>
>> This is an adaptation of
>> https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html
>>
>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
>> Link: https://lore.kernel.org/r/20210712155745.831580-2-mic@digikod.net
>
> Please see some comments below, mostly about formatting.
> The text looks good to me.
Thanks for the review.
>
> Thanks,
>
> Alex
>
>> ---
>>
>> Changes since v1:
>> * Replace all ".I" with ".IR", except when used for titles.
>
> Sorry, but I actually prefer the opposite: Use .I unless you really need
> .IR
When do we really need .IR? Isn't `.I "foo bar"` the same as `.IR foo
bar`? What do you use roman for?
Where can we find these preferences? The best I found was
https://www.man7.org/linux/man-pages/man7/groff_man.7.html but it
doesn't explain what to use. The current man pages seems to use both
interchangeably.
>
> If there was a misunderstanding about this, I'm sorry.
>
>> * Append punctuation to ".IR" and ".BR" when it makes sense (requested
>> by Alejandro Colomar).
>> * Cut lines according to the semantic newline rules (requested by
>> Alejandro Colomar).
>> * Remove roman style from ".TP" section titles (requested by Alejandro
>> Colomar).
>> * Add comma after "i.e." and "e.g.".
>> * Move the example in a new EXAMPLES section.
>> * Improve title.
>> * Explain the LSM acronym at first use.
>> ---
>> man7/landlock.7 | 356 ++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 356 insertions(+)
>> create mode 100644 man7/landlock.7
>>
>> diff --git a/man7/landlock.7 b/man7/landlock.7
>> new file mode 100644
>> index 000000000000..c89f5b1cabb6
>> --- /dev/null
>> +++ b/man7/landlock.7
>> @@ -0,0 +1,356 @@
>> +.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
>> +.\" Copyright © 2019-2020 ANSSI
>> +.\" Copyright © 2021 Microsoft Corporation
>> +.\"
>> +.\" %%%LICENSE_START(VERBATIM)
>> +.\" Permission is granted to make and distribute verbatim copies of this
>> +.\" manual provided the copyright notice and this permission notice are
>> +.\" preserved on all copies.
>> +.\"
>> +.\" Permission is granted to copy and distribute modified versions of
>> this
>> +.\" manual under the conditions for verbatim copying, provided that the
>> +.\" entire resulting derived work is distributed under the terms of a
>> +.\" permission notice identical to this one.
>> +.\"
>> +.\" Since the Linux kernel and libraries are constantly changing, this
>> +.\" manual page may be incorrect or out-of-date. The author(s)
>> assume no
>> +.\" responsibility for errors or omissions, or for damages resulting
>> from
>> +.\" the use of the information contained herein. The author(s) may not
>> +.\" have taken the same level of care in the production of this manual,
>> +.\" which is licensed free of charge, as they might when working
>> +.\" professionally.
>> +.\"
>> +.\" Formatted or processed versions of this manual, if unaccompanied by
>> +.\" the source, must acknowledge the copyright and authors of this work.
>> +.\" %%%LICENSE_END
>> +.\"
>> +.TH LANDLOCK 7 2021-06-27 Linux "Linux Programmer's Manual"
>> +.SH NAME
>> +Landlock \- unprivileged access-control
>> +.SH DESCRIPTION
>> +Landlock is an access-control system that enables any processes to //
>> securely /J/
Why adding a line break here? This line is 75 columns as stated by the
documentation: https://man7.org/linux/man-pages/man7/man-pages.7.html
I guess it helps for semantic newlines, right? If so, what are the rules?
>
> I'll add some line breaks [//] and line joins [/J/] through the email.
>
>> +restrict themselves and their future children.
>> +Because Landlock is a stackable Linux Security Module (LSM),
>> +it makes possible to create safe security sandboxes as new security
>> layers
>
> suggested wfix: "it makes it possible" or "it is possible"?
Ok
>
>> +in addition to the existing system-wide access-controls.
>> +This kind of sandbox is expected to help mitigate // the security
>> impact of /J/ > +bugs, // and unexpected or malicious behaviors in
>> applications.
>
> See line-break fixes above.
Ok
>
>> +.PP
>> +A Landlock security policy is a set of access rights
>> +(e.g., open a file in read-only, make a directory, etc.)
>> +tied to a file hierarchy.
>> +Such policy can be configured and enforced by processes for themselves
>> +using three system calls:
>> +.IP \(bu 2
>> +.BR landlock_create_ruleset (2)
>> +creates a new ruleset;
>> +.IP \(bu
>> +.BR landlock_add_rule (2)
>> +adds a new rule to a ruleset;
>> +.IP \(bu
>> +.BR landlock_restrict_self (2)
>> +enforces a ruleset on the calling thread.
>> +.PP
>> +To be able to use these system calls,
>> +the running kernel must support Landlock and // it must be enabled at
>> boot /J/
>> +time.
>
> See line-break fixes above
Ok
>
>> +.\"
>> +.SS Landlock rules
>> +A Landlock rule describes an action on an object.
>> +An object is currently a file hierarchy,
>> +and the related filesystem actions are defined with access rights (see
>> +.BR landlock_add_rule (2)).
>> +A set of rules is aggregated in a ruleset, // which can /J/
>> +then restrict the thread enforcing it, // and its future children.
>
> See line-break fixes above.
Ok
>
>> +.\"
>> +.SS Filesystem actions
>> +These flags enable to restrict a sandboxed process to a // set of
>> actions on /J/
>> +files and directories. > +Files or directories opened before the
>> sandboxing // are not subject
> to these /J/
>> +restrictions.
>
> See line-break fixes above.
Ok
>
>> +See
>> +.BR landlock_add_rule (2)
>> +and
>> +.BR landlock_create_ruleset (2)
>> +for more context.
>> +.PP
>> +A file can only receive these access rights:
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_EXECUTE
>> +Execute a file.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
>> +Open a file with write access.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_READ_FILE
>> +Open a file with read access.
>> +.PP
>> +A directory can receive access rights related to files or directories.
>> +The following access right is applied to the directory itself,
>> +and the directories beneath it:
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_READ_DIR
>> +Open a directory or list its content.
>> +.PP
>> +However,
>> +the following access rights only apply to the content of a directory,
>> +not the directory itself:
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_REMOVE_DIR
>> +Remove an empty directory or rename one.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_REMOVE_FILE
>> +Unlink (or rename) a file.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_CHAR
>> +Create (or rename or link) a character device.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_DIR
>> +Create (or rename) a directory.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_REG
>> +Create (or rename or link) a regular file.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_SOCK
>> +Create (or rename or link) a UNIX domain socket.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_FIFO
>> +Create (or rename or link) a named pipe.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_BLOCK
>> +Create (or rename or link) a block device.
>> +.TP
>> +.B LANDLOCK_ACCESS_FS_MAKE_SYM
>> +Create (or rename or link) a symbolic link.
>> +.\"
>> +.SS Layers of file path access rights
>> +Each time a thread enforces a ruleset on itself, // it updates its
>> Landlock /J/
>
> See line-break fixes above
Ok
>
>> +domain with a new layer of policy.
>> +Indeed, this complementary policy is composed with the potentially other
>> +rulesets already restricting this thread.
>> +A sandboxed thread can then safely add more constraints to itself with a
>> +new enforced ruleset.
>> +.PP
>> +One policy layer grants access to a file path // if at least one of
>> its rules /J/
>> +encountered on the path grants the access.
>> +A sandboxed thread can only access a file path // if all its enforced
>> policy /J/
>> +layers grant the access // as well as all the other system access
>> controls
>> +(e.g., filesystem DAC, other LSM policies, etc.).
>
> See line-break fixes above.
Ok
>
>> +.\"
>> +.SS Bind mounts and OverlayFS
>> +Landlock enables restricting access to file hierarchies,
>> +which means that these access rights can be propagated with bind mounts
>> +(cf.
>> +.BR mount_namespaces (7))
>> +but not with OverlayFS.
>> +.PP
>> +A bind mount mirrors a source file hierarchy to a destination.
>> +The destination hierarchy is then composed of the exact same files,
>> +on which Landlock rules can be tied, // either via the source or the /J/
>> +destination path.
>> +These rules restrict access when they are encountered on a path,
>> +which means that they can restrict access to // multiple file
>> hierarchies at /J/
>> +the same time,
>> +whether these hierarchies are the result of bind mounts or not.
>
>
> See line-break fixes above.
Ok
>
>> +.PP
>> +An OverlayFS mount point consists of upper and lower layers.
>> +These layers are combined in a merge directory, result of the mount
>> point.
>> +This merge hierarchy may include files from the upper and lower layers,
>> +but modifications performed on the merge hierarchy // only reflects
>> on the /J/
>
> s/reflects/reflect/
Ok
>
>> +upper layer.
>> +From a Landlock policy point of view,
>> +each OverlayFS layers and merge hierarchies are standalone and contains
>> +their own set of files and directories,
>> +which is different from bind mounts.
>
>
> Incorrect mix of singular and plural, I think.
Is it OK with s/contains/contain/?
>
>> +A policy restricting an OverlayFS layer will not restrict the resulted
>> +merged hierarchy, and vice versa.
>> +Landlock users should then only think about file hierarchies they
>> want to
>> +allow access to, regardless of the underlying filesystem.
>> +.\"
>> +.SS Inheritance
>> +Every new thread resulting from a
>> +.BR clone (2)
>> +inherits Landlock domain restrictions from its parent.
>> +This is similar to the
>> +.BR seccomp (2)
>> +inheritance or any other LSM dealing with task's
>
> Not sure:
>
> s/task/a task/
> or
> s/task's/tasks'/
I'll take "tasks'".
>
>> +.BR credentials (7).
>> +For instance, one process's thread may apply Landlock rules to itself,
>
> s/process's/process'/
As pointed out by Branden, this is correct.
>
>> +but they will not be automatically applied to other sibling threads
>> +(unlike POSIX thread credential changes, cf.
>> +.BR nptl (7)).
>> +.PP
>> +When a thread sandboxes itself, // we have the guarantee that the
>> related /J/
>> +security policy // will stay enforced on all this thread's descendants.
>> +This allows creating standalone and modular security policies // per /J/
>> +application,
>> +which will automatically be composed between themselves // according
>> to their /J/
>> +runtime parent policies.
>> +.\"
>> +.SS Ptrace restrictions
>> +A sandboxed process has less privileges than a non-sandboxed process and
>> +must then be subject to additional restrictions // when manipulating
>> another /J/
>> +process.
>> +To be allowed to use
>> +.BR ptrace (2)
>> +and related syscalls on a target process,
>> +a sandboxed process should have a subset of the target process rules,
>> +which means the tracee must be in a sub-domain of the tracer.
>> +.SH VERSIONS
>> +Landlock was added in Linux 5.13.
>> +.SH NOTES
>> +Landlock is enabled by CONFIG_SECURITY_LANDLOCK.
>
> .BR CONFIG_SECURITY_LANDLOCK .
Ok
>
>> +The
>> +.IR lsm=lsm1,...,lsmN
>
> s/.IR/.I/
Ok
>
>> +command line parameter controls the sequence of the initialization of
>> +Linux Security Modules.
>> +It must contain the string
>> +.IR landlock
>
> s/.IR/.I
Ok
>
>> +to enable Landlock.
>> +If the command line parameter is not specified,
>> +the initialization falls back to the value of the deprecated
>> +.IR security=
>
> s/.IR/.I/
Ok
>
>> +command line parameter and further to the value of CONFIG_LSM.
>> +We can check that Landlock is enabled by looking for
>> +.IR "landlock: Up and running."
>
> s/.IR/.I/
Ok
>
>> +in kernel logs.
>> +.PP
>> +It is currently not possible to restrict some file-related actions
>> +accessible through these syscall families:
>
> When using syscall to refer to system call (not the function syscall(2)),
> we use the extended form "system call".
Ok
>
>> +.BR chdir (2),
>> +.BR truncate (2),
>> +.BR stat (2),
>> +.BR flock (2),
>> +.BR chmod (2),
>> +.BR chown (2),
>> +.BR setxattr (2),
>> +.BR utime (2),
>> +.BR ioctl (2),
>> +.BR fcntl (2),
>> +.BR access (2).
>> +Future Landlock evolutions will enable to restrict them.
>> +.SH EXAMPLES
> I'd prefer a complete example with a main function, if you can come up
> with such one. If not, this will be ok.
I think it is clearer to not to use a full main to explain these basic
steps. A full working example is linked though.
>
>
>> +We first need to create the ruleset that will contain our rules.
>> +For this example,
>> +the ruleset will contain rules that only allow read actions,
>> +but write actions will be denied.
>> +The ruleset then needs to handle both of these kind of actions.
>> +See below for the description of filesystem actions.
>> +.PP
>> +.in +4n
>> +.EX
>> +int ruleset_fd;
>> +struct landlock_ruleset_attr ruleset_attr = {
>> + .handled_access_fs =
>> + LANDLOCK_ACCESS_FS_EXECUTE |
>> + LANDLOCK_ACCESS_FS_WRITE_FILE |
>> + LANDLOCK_ACCESS_FS_READ_FILE |
>> + LANDLOCK_ACCESS_FS_READ_DIR |
>> + LANDLOCK_ACCESS_FS_REMOVE_DIR |
>> + LANDLOCK_ACCESS_FS_REMOVE_FILE |
>> + LANDLOCK_ACCESS_FS_MAKE_CHAR |
>> + LANDLOCK_ACCESS_FS_MAKE_DIR |
>> + LANDLOCK_ACCESS_FS_MAKE_REG |
>> + LANDLOCK_ACCESS_FS_MAKE_SOCK |
>> + LANDLOCK_ACCESS_FS_MAKE_FIFO |
>> + LANDLOCK_ACCESS_FS_MAKE_BLOCK |
>> + LANDLOCK_ACCESS_FS_MAKE_SYM,
>> +};
>> +
>> +ruleset_fd = landlock_create_ruleset(&ruleset_attr,
>> sizeof(ruleset_attr), 0);
>> +if (ruleset_fd < 0) {
>> + perror("Failed to create a ruleset");
>> + return 1;
>> +}
>> +.EE
>> +.in
>> +.PP
>> +We can now add a new rule to this ruleset thanks to the returned file
>> +descriptor referring to this ruleset.
>> +The rule will only allow reading the file hierarchy
>> +.IR /usr .
Why ".IR" is correct here?
>> +Without another rule, write actions would then be denied by the ruleset.
>> +To add
>> +.IR /usr
>> +to the ruleset, we open it with the
>> +.IR O_PATH
>> +flag and fill the
>> +.IR "struct landlock_path_beneath_attr"
>> +with this file descriptor.
>> +.PP
>> +.in +4n
>> +.EX
>> +int err;
>> +struct landlock_path_beneath_attr path_beneath = {
>> + .allowed_access =
>> + LANDLOCK_ACCESS_FS_EXECUTE |
>> + LANDLOCK_ACCESS_FS_READ_FILE |
>> + LANDLOCK_ACCESS_FS_READ_DIR,
>> +};
>> +
>> +path_beneath.parent_fd = open("/usr", O_PATH | O_CLOEXEC);
>> +if (path_beneath.parent_fd < 0) {
>> + perror("Failed to open file");
>> + close(ruleset_fd);
>> + return 1;
>> +}
>> +err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
>> + &path_beneath, 0);
>> +close(path_beneath.parent_fd);
>> +if (err) {
>> + perror("Failed to update ruleset");
>> + close(ruleset_fd);
>> + return 1;
>> +}
>> +.EE
>> +.in
>> +.PP
>> +We now have a ruleset with one rule allowing read access to
>> +.IR /usr
>> +while denying all other handled accesses for the filesystem.
>> +The next step is to restrict the current thread from gaining more
>> +privileges
>> +(e.g., thanks to a set-user-ID binary).
>> +.PP
>> +.in +4n
>> +.EX
>> +if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
>> + perror("Failed to restrict privileges");
>> + close(ruleset_fd);
>> + return 1;
>> +}
>> +.EE
>> +.in
>> +.PP
>> +The current thread is now ready to sandbox itself with the ruleset.
>> +.PP
>> +.in +4n
>> +.EX
>> +if (landlock_restrict_self(ruleset_fd, 0)) {
>> + perror("Failed to enforce ruleset");
>> + close(ruleset_fd);
>> + return 1;
>> +}
>> +close(ruleset_fd);
>> +.EE
>> +.in
>> +.PP
>> +If the
>> +.BR landlock_restrict_self (2)
>> +system call succeeds, the current thread is now restricted and this
>> policy
>> +will be enforced on all its subsequently created children as well.
>> +Once a thread is landlocked, there is no way to remove its security
>> policy;
>> +only adding more restrictions is allowed.
>> +These threads are now in a new Landlock domain, // merge of their
>> parent one /J/
>> +(if any) with the new ruleset.
>> +.PP
>> +Full working code can be found in
>> +.UR
>> https://git.kernel.org\:/pub\:/scm\:/linux\:/kernel\:/git\:/stable\:/linux.git\:/tree\:/samples\:/landlock\:/sandboxer.c
>>
>> +.UE
>> +.SH SEE ALSO
>> +.BR landlock_create_ruleset (2),
>> +.BR landlock_add_rule (2),
>> +.BR landlock_restrict_self (2)
>> +.PP
>> +.UR https://landlock.io\:/
>> +.UE
>>
>
>
^ permalink raw reply
* Re: [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-07-30 11:45 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Stefan Berger, peterhuewe, jgg, linux-integrity,
linux-security-module, linux-kernel, Nayna Jain, George Wilson,
Nageswara R Sastry
In-Reply-To: <20210730005744.ph7x6nme5ngtpf43@kernel.org>
On 7/29/21 8:57 PM, Jarkko Sakkinen wrote:
> On Thu, Jul 29, 2021 at 09:39:18AM -0400, Stefan Berger wrote:
>> On 7/28/21 5:50 PM, Jarkko Sakkinen wrote:
>>> On Mon, Jul 26, 2021 at 11:00:51PM -0400, Stefan Berger wrote:
>>>> On 7/26/21 10:42 PM, Jarkko Sakkinen wrote:
>>>>> On Mon, Jul 12, 2021 at 12:25:05PM -0400, Stefan Berger wrote:
>>>>>> From: Stefan Berger <stefanb@linux.ibm.com>
>>>>>>
>>>>>> When rngd is run as root then lots of these types of message will appear
>>>>>> in the kernel log if the TPM has been configure to provide random bytes:
>>>>>>
>>>>>> [ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
>>>>>>
>>>>>> The issue is caused by the following call that is interrupted while
>>>>>> waiting for the TPM's response.
>>>>>>
>>>>>> sig = wait_event_interruptible(ibmvtpm->wq,
>>>>>> !ibmvtpm->tpm_processing_cmd);
>>>>>>
>>>>>> The solution is to use wait_event() instead.
>>>>> Why?
>>>> So it becomes uninterruptible and these error messages go away.
>>> We do not want to make a process uninterruptible. That would prevent
>>> killing it.
>> I guess we'll have to go back to this one then:
>> https://www.spinics.net/lists/linux-integrity/msg16741.html
> Makes a heck lot more sense.
>
> There's a typo in the commit message: PM_STATUS_BUSY
>
> Also the commit message lacks explanation of this change completely:
>
> @@ -690,8 +688,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> goto init_irq_cleanup;
> }
>
> - if (!strcmp(id->compat, "IBM,vtpm20")) {
> +
> + if (!strcmp(id->compat, "IBM,vtpm20"))
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto init_irq_cleanup;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> rc = tpm2_get_cc_attrs_tbl(chip);
>
> The last paragraph should be rewritten in imperative form.
will fix.
>
> Finally, you could simplify the fix by simply changing the type of
> tpm_processing_cmd to u8, and just set it to 'true' and 'false',
> which will set the first bit.
Are you sure? It's a bit mask we are using this with. Using 'true' for
these type of operations doesn't sound right.
u8 status = chip->ops->status(chip);
if ((status & chip->ops->req_complete_mask) ==
chip->ops->req_complete_val)
goto out_recv;
https://elixir.bootlin.com/linux/latest/source/drivers/char/tpm/tpm-interface.c#L108
@@ -457,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
.send = tpm_ibmvtpm_send,
.cancel = tpm_ibmvtpm_cancel,
.status = tpm_ibmvtpm_status,
- .req_complete_mask = 0,
+ .req_complete_mask = TPM_STATUS_BUSY,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
Stefan
>
> /Jarkko
^ permalink raw reply
* RE: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Roberto Sassu @ 2021-07-30 7:16 UTC (permalink / raw)
To: Mimi Zohar, gregkh@linuxfoundation.org, mchehab+huawei@kernel.org
Cc: linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <c9dffd9d29df095660beaa631ff252c4b33629a0.camel@linux.ibm.com>
> From: Mimi Zohar [mailto:zohar@linux.ibm.com]
> Sent: Thursday, July 29, 2021 11:21 PM
> Hi Roberto,
>
> On Mon, 2021-07-26 at 18:36 +0200, Roberto Sassu wrote:
> > /*
> > + * digest_list_read: read and parse the digest list from the path
> > + */
> > +static ssize_t digest_list_read(char *path, enum ops op)
> > +{
> > + void *data = NULL;
> > + char *datap;
> > + size_t size;
> > + u8 actions = 0;
> > + struct file *file;
> > + char event_name[NAME_MAX + 9 + 1];
> > + u8 digest[IMA_MAX_DIGEST_SIZE] = { 0 };
> > + enum hash_algo algo;
> > + int rc, pathlen = strlen(path);
> > +
> > + /* Remove \n. */
> > + datap = path;
> > + strsep(&datap, "\n");
> > +
> > + file = filp_open(path, O_RDONLY, 0);
> > + if (IS_ERR(file)) {
> > + pr_err("unable to open file: %s (%ld)", path, PTR_ERR(file));
> > + return PTR_ERR(file);
> > + }
> > +
> > + rc = kernel_read_file(file, 0, &data, INT_MAX, NULL,
> > + READING_DIGEST_LIST);
> > + if (rc < 0) {
> > + pr_err("unable to read file: %s (%d)", path, rc);
> > + goto out;
> > + }
> > +
> > + size = rc;
> > +
> > + snprintf(event_name, sizeof(event_name), "%s_file_%s",
> > + op == DIGEST_LIST_ADD ? "add" : "del",
> > + file_dentry(file)->d_name.name);
> > +
> > + rc = ima_measure_critical_data("diglim", event_name, data, size, false,
> > + digest, sizeof(digest));
> > + if (rc < 0 && rc != -EEXIST)
> > + goto out_vfree;
>
> The digest lists could easily be measured while reading the digest list
> file above in kernel_read_file(). What makes it "critical-data"? In
> the SELinux case, the in memory SELinux policy is being measured and
> re-measured to make sure it hasn't been modified. Is the digest list
> file data being measured more than once?
Hi Mimi
yes, the digest lists can be measured with kernel_read_file().
I didn't send the change yet, but I added a DIGEST_LIST_CHECK
hook mapped to READING_DIGEST_LIST, so that digest lists
can be easily measured or appraised.
The point was that the digest of the digest list must be always
calculated, as it is added to the hash table. Instead of duplicating
the code, I preferred to use ima_measure_critical_data().
The advantage is also that, if the use case is to just measure
digest lists, ima_measure_critical_data() could do both at the
same time.
Digest lists can be seen as "critical data" in the sense that
they can affect the security decision on whether to grant
access to a file or not, assuming that an appropriate rule is
added in the IMA policy.
> I understand that with your changes to ima_measure_critical_data(),
> which are now in next-integrity-testing branch, allow IMA to calculate
> the file data hash.
Yes, correct. But actually there is another useful use case.
If digest lists are not in the format supported by the kernel,
the user space parser has to convert them before uploading
them to the kernel.
ima_measure_critical_data() would in this case measure
the converted digest list (it is written directly, without
sending the file path). It is easier to attest the result,
instead of determining whether the user space parser
produced the expected result (by checking the files it
read).
Thanks
Roberto
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Li Jian, Shi Yanli
> thanks,
>
> Mimi
>
> > +
> > + algo = ima_get_current_hash_algo();
> > +
>
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: fix use as module when CONFIG_TCG_TPM=m
From: Ahmad Fatoum @ 2021-07-30 6:21 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
Sumit Garg, David Howells, Herbert Xu, David S. Miller, kernel,
Andreas Rammhold, David Gstir, Richard Weinberger, keyrings,
linux-crypto, linux-kernel, linux-security-module,
linux-integrity
In-Reply-To: <20210730002101.2tcb3bs2lxdvmuqk@kernel.org>
On 30.07.21 02:31, Jarkko Sakkinen wrote:
> On Thu, Jul 29, 2021 at 12:29:38AM +0200, Ahmad Fatoum wrote:
>> On 28.07.21 23:52, Jarkko Sakkinen wrote:
>>> On Tue, Jul 27, 2021 at 06:24:49AM +0200, Ahmad Fatoum wrote:
>>>> On 27.07.21 05:04, Jarkko Sakkinen wrote:
>>>>>> Reported-by: Andreas Rammhold <andreas@rammhold.de>
>>>>>> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
>>>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>>>
>>>>> Is it absolutely need to do all this *just* to fix the bug?
>>>>>
>>>>> For a pure bug fix the most essential thing is to be able the backport
>>>>> it to stable kernels.
>>>>
>>>> Not much happened in-between, so a backport should be trivial.
>>>> I can provide these if needed.
>>>
>>> "not much" is not good enough. It should be "not anything".
>>
>> "Not much" [code that could conflict was added in-between].
>>
>> I just checked and it applies cleanly on v5.13. On the off chance
>> that this patch conflicts with another stable backport by the time
>> it's backported, I'll get a friendly automated email and send out
>> a rebased patch.
>
> What you should do is to split this into patch that exactly
> fixes the issue, and to one that adds the "niceties".
I'd rather not send out patches I believe to be incomplete, sorry.
If you want to fix the regression's root cause of insufficient
Kconfig description, that here is what it takes.
You can take Andreas' regression fix for stable and replace it with my patch later.
I'll send out a new version removing references that it fixes a regression.
Cheers,
Ahmad
>
> /Jarkko
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v3] KEYS: trusted: Fix trusted key backends when building as module
From: Ahmad Fatoum @ 2021-07-30 6:14 UTC (permalink / raw)
To: Andreas Rammhold, James Bottomley, Jarkko Sakkinen, Mimi Zohar,
David Howells, James Morris, Serge E. Hallyn, Sumit Garg
Cc: linux-integrity, keyrings, linux-security-module, linux-kernel
In-Reply-To: <20210730012822.3460913-1-andreas@rammhold.de>
On 30.07.21 03:28, Andreas Rammhold wrote:
> Before this commit the kernel could end up with no trusted key sources
> even though both of the currently supported backends (TPM and TEE) were
> compiled as modules. This manifested in the trusted key type not being
> registered at all.
>
> When checking if a CONFIG_… preprocessor variable is defined we only
> test for the builtin (=y) case and not the module (=m) case. By using
> the IS_REACHABLE() macro we do test for both cases.
>
> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>
> ---
>
> v3:
> * Fixed patch formatting
>
> v2:
> * Fixed commit message
> * Switched from IS_DEFINED() to IS_REACHABLE()
>
> security/keys/trusted-keys/trusted_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..5b35f1b87644 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
> MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>
> static const struct trusted_key_source trusted_key_sources[] = {
> -#if defined(CONFIG_TCG_TPM)
> +#if IS_REACHABLE(CONFIG_TCG_TPM)
> { "tpm", &trusted_key_tpm_ops },
> #endif
> -#if defined(CONFIG_TEE)
> +#if IS_REACHABLE(CONFIG_TEE)
> { "tee", &trusted_key_tee_ops },
> #endif
> };
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* [PATCH] apparmor: use per file locks for transactional queries
From: Hamza Mahfooz @ 2021-07-30 5:23 UTC (permalink / raw)
To: linux-kernel
Cc: Hamza Mahfooz, John Johansen, James Morris, Serge E. Hallyn,
linux-security-module
As made mention of in commit 1dea3b41e84c5 ("apparmor: speed up
transactional queries"), a single lock is currently used to synchronize
transactional queries. We can, use the lock allocated for each file by
VFS instead.
Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
---
security/apparmor/apparmorfs.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/security/apparmor/apparmorfs.c b/security/apparmor/apparmorfs.c
index 2ee3b3d29f10..c0b626a271a0 100644
--- a/security/apparmor/apparmorfs.c
+++ b/security/apparmor/apparmorfs.c
@@ -812,8 +812,6 @@ struct multi_transaction {
};
#define MULTI_TRANSACTION_LIMIT (PAGE_SIZE - sizeof(struct multi_transaction))
-/* TODO: replace with per file lock */
-static DEFINE_SPINLOCK(multi_transaction_lock);
static void multi_transaction_kref(struct kref *kref)
{
@@ -847,10 +845,10 @@ static void multi_transaction_set(struct file *file,
AA_BUG(n > MULTI_TRANSACTION_LIMIT);
new->size = n;
- spin_lock(&multi_transaction_lock);
+ spin_lock(&file->f_lock);
old = (struct multi_transaction *) file->private_data;
file->private_data = new;
- spin_unlock(&multi_transaction_lock);
+ spin_unlock(&file->f_lock);
put_multi_transaction(old);
}
@@ -879,9 +877,10 @@ static ssize_t multi_transaction_read(struct file *file, char __user *buf,
struct multi_transaction *t;
ssize_t ret;
- spin_lock(&multi_transaction_lock);
+ spin_lock(&file->f_lock);
t = get_multi_transaction(file->private_data);
- spin_unlock(&multi_transaction_lock);
+ spin_unlock(&file->f_lock);
+
if (!t)
return 0;
--
2.32.0
^ permalink raw reply related
* Re: [PATCH v3] KEYS: trusted: Fix trusted key backends when building as module
From: Sumit Garg @ 2021-07-30 4:54 UTC (permalink / raw)
To: Andreas Rammhold
Cc: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
James Morris, Serge E. Hallyn, Ahmad Fatoum, linux-integrity,
open list:ASYMMETRIC KEYS, open list:SECURITY SUBSYSTEM,
Linux Kernel Mailing List
In-Reply-To: <20210730012822.3460913-1-andreas@rammhold.de>
On Fri, 30 Jul 2021 at 06:58, Andreas Rammhold <andreas@rammhold.de> wrote:
>
> Before this commit the kernel could end up with no trusted key sources
> even though both of the currently supported backends (TPM and TEE) were
> compiled as modules. This manifested in the trusted key type not being
> registered at all.
>
> When checking if a CONFIG_… preprocessor variable is defined we only
> test for the builtin (=y) case and not the module (=m) case. By using
> the IS_REACHABLE() macro we do test for both cases.
>
> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> ---
>
> v3:
> * Fixed patch formatting
>
> v2:
> * Fixed commit message
> * Switched from IS_DEFINED() to IS_REACHABLE()
>
> security/keys/trusted-keys/trusted_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
LGTM.
Reviewed-by: Sumit Garg <sumit.garg@linaro.org>
-Sumit
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..5b35f1b87644 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
> MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>
> static const struct trusted_key_source trusted_key_sources[] = {
> -#if defined(CONFIG_TCG_TPM)
> +#if IS_REACHABLE(CONFIG_TCG_TPM)
> { "tpm", &trusted_key_tpm_ops },
> #endif
> -#if defined(CONFIG_TEE)
> +#if IS_REACHABLE(CONFIG_TEE)
> { "tee", &trusted_key_tee_ops },
> #endif
> };
> --
> 2.32.0
>
^ permalink raw reply
* Re: [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Nageswara Sastry @ 2021-07-30 4:15 UTC (permalink / raw)
To: Stefan Berger, jarkko
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger, Nayna Jain, George Wilson
In-Reply-To: <20210729161931.3609894-1-stefanb@linux.vnet.ibm.com>
On 29/07/21 9:49 pm, Stefan Berger wrote:
> From: Stefan Berger <stefanb@linux.ibm.com>
>
> When rngd is run as root then lots of these types of message will appear
> in the kernel log if the TPM has been configure to provide random bytes:
>
> [ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
>
> The issue is caused by the following call that is interrupted while
> waiting for the TPM's response.
>
> sig = wait_event_interruptible(ibmvtpm->wq,
> !ibmvtpm->tpm_processing_cmd);
>
> Rather than waiting for the response in the low level driver, have it use
> the polling loop in tpm_try_transmit() that uses a command's duration to
> poll until a result has been returned by the TPM, thus ending when the
> timeout has occurred but not responding to signals and ctrl-c anymore.
> To stay in this polling loop we now extend tpm_ibmvtpm_status() to return
> PM_STATUS_BUSY for as long as the vTPM is busy. Since we will need the
> timeouts in this loop now we get the TPM 1.2 and TPM 2 timeouts with
> tpm_get_timeouts().
>
> Rename the tpm_processing_cmd to tpm_status in ibmvtpm_dev and set the
> TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
>
> To recreat the issue start rngd like this:
>
> sudo rngd -r /dev/hwrng -t
>
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1981473
> Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
> Cc: Nayna Jain <nayna@linux.ibm.com>
> Cc: George Wilson <gcwilson@linux.ibm.com>
> Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
> Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Tested with
rngd -r /dev/hwrng -t
rngd -r /dev/tpm0 -t
There are no tpm errors seen.
Thank you.
> ---
> drivers/char/tpm/tpm_ibmvtpm.c | 31 ++++++++++++++++++-------------
> drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
> 2 files changed, 20 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
> index 903604769de9..5d795866b483 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.c
> +++ b/drivers/char/tpm/tpm_ibmvtpm.c
> @@ -106,17 +106,12 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
> {
> struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
> u16 len;
> - int sig;
>
> if (!ibmvtpm->rtce_buf) {
> dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
> return 0;
> }
>
> - sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
> - if (sig)
> - return -EINTR;
> -
> len = ibmvtpm->res_len;
>
> if (count < len) {
> @@ -220,11 +215,12 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> return -EIO;
> }
>
> - if (ibmvtpm->tpm_processing_cmd) {
> + if ((ibmvtpm->tpm_status & TPM_STATUS_BUSY)) {
> dev_info(ibmvtpm->dev,
> "Need to wait for TPM to finish\n");
> /* wait for previous command to finish */
> - sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
> + sig = wait_event_interruptible(ibmvtpm->wq,
> + (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
> if (sig)
> return -EINTR;
> }
> @@ -237,7 +233,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> * set the processing flag before the Hcall, since we may get the
> * result (interrupt) before even being able to check rc.
> */
> - ibmvtpm->tpm_processing_cmd = true;
> + ibmvtpm->tpm_status |= TPM_STATUS_BUSY;
>
> again:
> rc = ibmvtpm_send_crq(ibmvtpm->vdev,
> @@ -255,7 +251,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
> goto again;
> }
> dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
> - ibmvtpm->tpm_processing_cmd = false;
> + ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
> }
>
> spin_unlock(&ibmvtpm->rtce_lock);
> @@ -269,7 +265,9 @@ static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)
>
> static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
> {
> - return 0;
> + struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
> +
> + return ibmvtpm->tpm_status;
> }
>
> /**
> @@ -457,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
> .send = tpm_ibmvtpm_send,
> .cancel = tpm_ibmvtpm_cancel,
> .status = tpm_ibmvtpm_status,
> - .req_complete_mask = 0,
> + .req_complete_mask = TPM_STATUS_BUSY,
> .req_complete_val = 0,
> .req_canceled = tpm_ibmvtpm_req_canceled,
> };
> @@ -550,7 +548,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
> case VTPM_TPM_COMMAND_RES:
> /* len of the data in rtce buffer */
> ibmvtpm->res_len = be16_to_cpu(crq->len);
> - ibmvtpm->tpm_processing_cmd = false;
> + ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
> wake_up_interruptible(&ibmvtpm->wq);
> return;
> default:
> @@ -688,8 +686,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
> goto init_irq_cleanup;
> }
>
> - if (!strcmp(id->compat, "IBM,vtpm20")) {
> +
> + if (!strcmp(id->compat, "IBM,vtpm20"))
> chip->flags |= TPM_CHIP_FLAG_TPM2;
> +
> + rc = tpm_get_timeouts(chip);
> + if (rc)
> + goto init_irq_cleanup;
> +
> + if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> rc = tpm2_get_cc_attrs_tbl(chip);
> if (rc)
> goto init_irq_cleanup;
> diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> index b92aa7d3e93e..252f1cccdfc5 100644
> --- a/drivers/char/tpm/tpm_ibmvtpm.h
> +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
> wait_queue_head_t wq;
> u16 res_len;
> u32 vtpm_version;
> - bool tpm_processing_cmd;
> + u8 tpm_status;
> +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
> };
>
> #define CRQ_RES_BUF_SIZE PAGE_SIZE
>
--
Thanks and Regards
R.Nageswara Sastry
^ permalink raw reply
* [PATCH v3] KEYS: trusted: Fix trusted key backends when building as module
From: Andreas Rammhold @ 2021-07-30 1:28 UTC (permalink / raw)
To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
James Morris, Serge E. Hallyn, Sumit Garg
Cc: Ahmad Fatoum, Andreas Rammhold, linux-integrity, keyrings,
linux-security-module, linux-kernel
Before this commit the kernel could end up with no trusted key sources
even though both of the currently supported backends (TPM and TEE) were
compiled as modules. This manifested in the trusted key type not being
registered at all.
When checking if a CONFIG_… preprocessor variable is defined we only
test for the builtin (=y) case and not the module (=m) case. By using
the IS_REACHABLE() macro we do test for both cases.
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v3:
* Fixed patch formatting
v2:
* Fixed commit message
* Switched from IS_DEFINED() to IS_REACHABLE()
security/keys/trusted-keys/trusted_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index d5c891d8d353..5b35f1b87644 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
static const struct trusted_key_source trusted_key_sources[] = {
-#if defined(CONFIG_TCG_TPM)
+#if IS_REACHABLE(CONFIG_TCG_TPM)
{ "tpm", &trusted_key_tpm_ops },
#endif
-#if defined(CONFIG_TEE)
+#if IS_REACHABLE(CONFIG_TEE)
{ "tee", &trusted_key_tee_ops },
#endif
};
--
2.32.0
^ permalink raw reply related
* Re: [PATCH v2] KEYS: trusted: Fix trusted key backends when building as module
From: Jarkko Sakkinen @ 2021-07-30 1:02 UTC (permalink / raw)
To: Andreas Rammhold
Cc: James Bottomley, Mimi Zohar, David Howells, James Morris,
Serge E. Hallyn, Sumit Garg, Ahmad Fatoum, linux-integrity,
keyrings, linux-security-module, linux-kernel
In-Reply-To: <20210729183333.1070629-1-andreas@rammhold.de>
On Thu, Jul 29, 2021 at 08:33:32PM +0200, Andreas Rammhold wrote:
> Before this commit the kernel could end up with no trusted key sources
> even though both of the currently supported backends (TPM and TEE) were
> compiled as modules. This manifested in the trusted key type not being
> registered at all.
>
> When checking if a CONFIG_… preprocessor variable is defined we only
> test for the builtin (=y) case and not the module (=m) case. By using
> the IS_REACHABLE() macro we do test for both cases.
>
>
> v2:
> * Fixed commit message
> * Switched from IS_DEFINED() to IS_REACHABLE()
Please put these below '---' because otherwise they included to the commit
log.
> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
> ---
>
> Here is the version that was proposed by Ahmad [1] in response to the
> feedback received in the "[PATCH v2] KEYS: trusted: fix use as module
> when CONFIG_TCG_TPM=m" discussion [2].
>
> I have tested both of the patches on v5.13 and they both fix the problem
> I originally encountered.
>
> [1] https://lore.kernel.org/keyrings/fe39a449-88df-766b-a13a-290f4847d43e@pengutronix.de/
> [2] https://lore.kernel.org/keyrings/20210721160258.7024-1-a.fatoum@pengutronix.de/
>
>
> security/keys/trusted-keys/trusted_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index d5c891d8d353..5b35f1b87644 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
> MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
>
> static const struct trusted_key_source trusted_key_sources[] = {
> -#if defined(CONFIG_TCG_TPM)
> +#if IS_REACHABLE(CONFIG_TCG_TPM)
> { "tpm", &trusted_key_tpm_ops },
> #endif
> -#if defined(CONFIG_TEE)
> +#if IS_REACHABLE(CONFIG_TEE)
> { "tee", &trusted_key_tee_ops },
> #endif
> };
> --
> 2.32.0
>
>
Can you send a one more v3 without the changelog in the commit message
and also add
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
/Jarkko
^ permalink raw reply
* Re: [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Jarkko Sakkinen @ 2021-07-30 0:57 UTC (permalink / raw)
To: Stefan Berger
Cc: Stefan Berger, peterhuewe, jgg, linux-integrity,
linux-security-module, linux-kernel, Nayna Jain, George Wilson,
Nageswara R Sastry
In-Reply-To: <2add3eac-916e-5072-f62d-23c65e23fb17@linux.ibm.com>
On Thu, Jul 29, 2021 at 09:39:18AM -0400, Stefan Berger wrote:
>
> On 7/28/21 5:50 PM, Jarkko Sakkinen wrote:
> > On Mon, Jul 26, 2021 at 11:00:51PM -0400, Stefan Berger wrote:
> > > On 7/26/21 10:42 PM, Jarkko Sakkinen wrote:
> > > > On Mon, Jul 12, 2021 at 12:25:05PM -0400, Stefan Berger wrote:
> > > > > From: Stefan Berger <stefanb@linux.ibm.com>
> > > > >
> > > > > When rngd is run as root then lots of these types of message will appear
> > > > > in the kernel log if the TPM has been configure to provide random bytes:
> > > > >
> > > > > [ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
> > > > >
> > > > > The issue is caused by the following call that is interrupted while
> > > > > waiting for the TPM's response.
> > > > >
> > > > > sig = wait_event_interruptible(ibmvtpm->wq,
> > > > > !ibmvtpm->tpm_processing_cmd);
> > > > >
> > > > > The solution is to use wait_event() instead.
> > > > Why?
> > > So it becomes uninterruptible and these error messages go away.
> > We do not want to make a process uninterruptible. That would prevent
> > killing it.
>
> I guess we'll have to go back to this one then:
> https://www.spinics.net/lists/linux-integrity/msg16741.html
Makes a heck lot more sense.
There's a typo in the commit message: PM_STATUS_BUSY
Also the commit message lacks explanation of this change completely:
@@ -690,8 +688,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}
- if (!strcmp(id->compat, "IBM,vtpm20")) {
+
+ if (!strcmp(id->compat, "IBM,vtpm20"))
chip->flags |= TPM_CHIP_FLAG_TPM2;
+
+ rc = tpm_get_timeouts(chip);
+ if (rc)
+ goto init_irq_cleanup;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_get_cc_attrs_tbl(chip);
The last paragraph should be rewritten in imperative form.
Finally, you could simplify the fix by simply changing the type of
tpm_processing_cmd to u8, and just set it to 'true' and 'false',
which will set the first bit.
/Jarkko
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: fix use as module when CONFIG_TCG_TPM=m
From: Jarkko Sakkinen @ 2021-07-30 0:31 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
Sumit Garg, David Howells, Herbert Xu, David S. Miller, kernel,
Andreas Rammhold, David Gstir, Richard Weinberger, keyrings,
linux-crypto, linux-kernel, linux-security-module,
linux-integrity
In-Reply-To: <f0f05df9-95bb-8b67-cecc-742af0b19f1e@pengutronix.de>
On Thu, Jul 29, 2021 at 12:29:38AM +0200, Ahmad Fatoum wrote:
> On 28.07.21 23:52, Jarkko Sakkinen wrote:
> > On Tue, Jul 27, 2021 at 06:24:49AM +0200, Ahmad Fatoum wrote:
> >> On 27.07.21 05:04, Jarkko Sakkinen wrote:
> >>>> Reported-by: Andreas Rammhold <andreas@rammhold.de>
> >>>> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
> >>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> >>>
> >>> Is it absolutely need to do all this *just* to fix the bug?
> >>>
> >>> For a pure bug fix the most essential thing is to be able the backport
> >>> it to stable kernels.
> >>
> >> Not much happened in-between, so a backport should be trivial.
> >> I can provide these if needed.
> >
> > "not much" is not good enough. It should be "not anything".
>
> "Not much" [code that could conflict was added in-between].
>
> I just checked and it applies cleanly on v5.13. On the off chance
> that this patch conflicts with another stable backport by the time
> it's backported, I'll get a friendly automated email and send out
> a rebased patch.
What you should do is to split this into patch that exactly
fixes the issue, and to one that adds the "niceties".
/Jarkko
^ permalink raw reply
* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Alejandro Colomar (man-pages) @ 2021-07-29 22:34 UTC (permalink / raw)
To: G. Branden Robinson, Mickaël Salaün
Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
Vincent Dagonneau, landlock, linux-kernel, linux-man,
linux-security-module, Mickaël Salaün, Michael Kerrisk
In-Reply-To: <20210729220129.ymfdnybbpvej4qck@localhost.localdomain>
Hi Branden!
On 7/30/21 12:01 AM, G. Branden Robinson wrote:
> Hi, Alex!
>
> [regrets for the huge CC--those not interested in English/linux-man
> style issues can skip this]
>
> At 2021-07-29T16:56:37+0200, Alejandro Colomar (man-pages) wrote:
>> On 7/12/21 5:57 PM, Mickaël Salaün wrote:
>>> +For instance, one process's thread may apply Landlock rules to itself,
>>
>> s/process's/process'/
>
> Many English language authorities would disagree with you, but I'll skip
> digging up citations to them because the Linux man-pages project's
> practice is already firmly in the other direction.
>
> $ git grep "s's\>" | wc -l
> 322
>
> Moreover, "process's" is extensively attested as most of those...
>
> $ git grep "process's" | wc -l
> 320
My bad. It was correct. I was wrong.
I learnt today that the omission of "s" after the apostrophe is only in
the case of plural nouns (I don't remember having learnt that at school :/).
I suspect I probably wrote that before learning that.
>
> ...and a global change in the opposite direction from your
> recommendation is credited to mtk in the Changes.old file.
>
> $ grep -B2 "process' " Changes.old |head -n 3
> A few files
> mtk
> s/process' /process's/
>
> Finding examples of the opposite practice is complicated by the use of
> apostrophes as single quotes (these usually _aren't_ confounded by code
> examples, however, since it would be incorrect C language syntax to
> quote a string literal with them). There are many such occurrences in
> Changes.old; I'll skip them. The remainder are few enough that I'll
> quote them here.
>
> $ git grep -E "s'(\s|$)" man*
> man2/adjtimex.2:Linux uses David L.\& Mills' clock adjustment algorithm (see RFC\ 5905).
> man2/move_pages.2:.\" FIXME Describe the result if pointers in the 'pages' array are
> man2/utimensat.2:.\" given a 'times' array in which both tv_nsec fields are UTIME_NOW, which
> man2/utimensat.2:.\" provides equivalent functionality to specifying 'times' as NULL, the
> man3/getaddrinfo.3:.\" 2008-02-26, mtk; clarify discussion of NULL 'hints' argument; other
> man3/printf.3:thousands' grouping character is used.
> man3/printf.3:the output is to be grouped with thousands' grouping characters
> man3/printf.3:.\" no thousands' separator, no NaN or infinity, no "%m$" and "*m$".
> man3/scanf.3:This specifies that the input number may include thousands'
> man3/xdr.3:the array elements' C form, and their external
> man3/xdr.3:the array elements' C form, and their external
> man5/elf.5:The array element is unused and the other members' values are undefined.
> man5/proc.5:under the default overcommit 'guess' mode (i.e., 0 in
> man5/proc.5:because other nodes' memory may be free,
> man7/bootparam.7:The Linux kernel accepts certain 'command-line options' or 'boot time
> man7/bootparam.7:parameters' at the moment it is started.
> man7/bootparam.7:The option 'reboot=bios' will
> man7/bootparam.7:A SCSI device can have a number of 'subdevices' contained within
> man7/hier.7:Users' mailboxes.
> man7/mount_namespaces.7:the root directory under several users' home directories.
> man7/uri.7:schemes; see those tools' documentation for information on those schemes.
> man7/uri.7:detects the users' environment (e.g., text or graphics,
> man8/ld.so.8:and do not apply to those objects' children,
>
> Of the above,
>
> 1. most are correct uses of the English plural possessive ("nodes'");
> 2. a few occur in comments, where they're fine if present as
> commentary--if they're "commented out" chunks of man page source,
> they should follow man page formatting rules in the event they
> require "resurrection";
> 3. we see some uses of apostrophes as quotation marks; and
> 4. David L. Mills's name is marked as a plural possessive. The
> application of apostrophe+s to singular proper names ending in "s" is
> a debated issue, and there is probably some room for personal
> preference on the part of the bearer of the name.
>
> Two side issues:
>
> A. Regarding point 3, I'd say this illustrates advantages of using
> special character escape sequences like \[lq] and \[rq] for quotation.
> First, you will get paired quotation marks in UTF-8, PDF, and HTML
> output. Second, you won't encounter false positives in searches like
> the above. Third, you semantically enrich the content. On the
> downside, adopting special character escapes would likely mean having to
> choose between U.S. and U.K. quotation styles[1].
I don't know what to do about this. For searches, if you come up with a
complex enough regex, you can get rid of quotations. If we use
different characters, then it will be really difficult to search for
actual quotations (I don't have them on my keyboard ;).
But having nicer PDF/HTML pages would be an advantage. However, I think
most usage of man-pages is in the terminal, so I'd focus on the terminal.
What do you think about this?
>
> B. Regarding another active thread we're in, I observe
>
> man2/adjtimex.2:Linux uses David L.\& Mills' clock adjustment algorithm (see RFC\ 5905).
>
> as another case where \~ recommends itself over "\ "; this isn't even a
> code example, and it illustrates the desirability of decoupling
> non-breaking from participation in space adjustment.
Agreed.
>
> Popping the stack, have I persuaded you on the plural possessive front?
> :)
Yup :)
>
> Best regards,
> Branden
>
> [1] https://man7.org/linux/man-pages/man7/groff_char.7.html (search for
> "the apostrophe")
>
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
^ permalink raw reply
* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: G. Branden Robinson @ 2021-07-29 22:01 UTC (permalink / raw)
To: Alejandro Colomar (man-pages)
Cc: Mickaël Salaün, Jann Horn, Jonathan Corbet, Kees Cook,
Randy Dunlap, Vincent Dagonneau, landlock, linux-kernel,
linux-man, linux-security-module, Mickaël Salaün,
Michael Kerrisk
In-Reply-To: <3f1b943b-2477-2c4e-c835-d6616888176c@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4651 bytes --]
Hi, Alex!
[regrets for the huge CC--those not interested in English/linux-man
style issues can skip this]
At 2021-07-29T16:56:37+0200, Alejandro Colomar (man-pages) wrote:
> On 7/12/21 5:57 PM, Mickaël Salaün wrote:
> > +For instance, one process's thread may apply Landlock rules to itself,
>
> s/process's/process'/
Many English language authorities would disagree with you, but I'll skip
digging up citations to them because the Linux man-pages project's
practice is already firmly in the other direction.
$ git grep "s's\>" | wc -l
322
Moreover, "process's" is extensively attested as most of those...
$ git grep "process's" | wc -l
320
...and a global change in the opposite direction from your
recommendation is credited to mtk in the Changes.old file.
$ grep -B2 "process' " Changes.old |head -n 3
A few files
mtk
s/process' /process's/
Finding examples of the opposite practice is complicated by the use of
apostrophes as single quotes (these usually _aren't_ confounded by code
examples, however, since it would be incorrect C language syntax to
quote a string literal with them). There are many such occurrences in
Changes.old; I'll skip them. The remainder are few enough that I'll
quote them here.
$ git grep -E "s'(\s|$)" man*
man2/adjtimex.2:Linux uses David L.\& Mills' clock adjustment algorithm (see RFC\ 5905).
man2/move_pages.2:.\" FIXME Describe the result if pointers in the 'pages' array are
man2/utimensat.2:.\" given a 'times' array in which both tv_nsec fields are UTIME_NOW, which
man2/utimensat.2:.\" provides equivalent functionality to specifying 'times' as NULL, the
man3/getaddrinfo.3:.\" 2008-02-26, mtk; clarify discussion of NULL 'hints' argument; other
man3/printf.3:thousands' grouping character is used.
man3/printf.3:the output is to be grouped with thousands' grouping characters
man3/printf.3:.\" no thousands' separator, no NaN or infinity, no "%m$" and "*m$".
man3/scanf.3:This specifies that the input number may include thousands'
man3/xdr.3:the array elements' C form, and their external
man3/xdr.3:the array elements' C form, and their external
man5/elf.5:The array element is unused and the other members' values are undefined.
man5/proc.5:under the default overcommit 'guess' mode (i.e., 0 in
man5/proc.5:because other nodes' memory may be free,
man7/bootparam.7:The Linux kernel accepts certain 'command-line options' or 'boot time
man7/bootparam.7:parameters' at the moment it is started.
man7/bootparam.7:The option 'reboot=bios' will
man7/bootparam.7:A SCSI device can have a number of 'subdevices' contained within
man7/hier.7:Users' mailboxes.
man7/mount_namespaces.7:the root directory under several users' home directories.
man7/uri.7:schemes; see those tools' documentation for information on those schemes.
man7/uri.7:detects the users' environment (e.g., text or graphics,
man8/ld.so.8:and do not apply to those objects' children,
Of the above,
1. most are correct uses of the English plural possessive ("nodes'");
2. a few occur in comments, where they're fine if present as
commentary--if they're "commented out" chunks of man page source,
they should follow man page formatting rules in the event they
require "resurrection";
3. we see some uses of apostrophes as quotation marks; and
4. David L. Mills's name is marked as a plural possessive. The
application of apostrophe+s to singular proper names ending in "s" is
a debated issue, and there is probably some room for personal
preference on the part of the bearer of the name.
Two side issues:
A. Regarding point 3, I'd say this illustrates advantages of using
special character escape sequences like \[lq] and \[rq] for quotation.
First, you will get paired quotation marks in UTF-8, PDF, and HTML
output. Second, you won't encounter false positives in searches like
the above. Third, you semantically enrich the content. On the
downside, adopting special character escapes would likely mean having to
choose between U.S. and U.K. quotation styles[1].
B. Regarding another active thread we're in, I observe
man2/adjtimex.2:Linux uses David L.\& Mills' clock adjustment algorithm (see RFC\ 5905).
as another case where \~ recommends itself over "\ "; this isn't even a
code example, and it illustrates the desirability of decoupling
non-breaking from participation in space adjustment.
Popping the stack, have I persuaded you on the plural possessive front?
:)
Best regards,
Branden
[1] https://man7.org/linux/man-pages/man7/groff_char.7.html (search for
"the apostrophe")
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [RFC][PATCH v2 06/12] diglim: Interfaces - digest_list_add, digest_list_del
From: Mimi Zohar @ 2021-07-29 21:20 UTC (permalink / raw)
To: Roberto Sassu, gregkh, mchehab+huawei
Cc: linux-integrity, linux-security-module, linux-doc,
linux-kselftest, linux-kernel
In-Reply-To: <20210726163700.2092768-7-roberto.sassu@huawei.com>
Hi Roberto,
On Mon, 2021-07-26 at 18:36 +0200, Roberto Sassu wrote:
> /*
> + * digest_list_read: read and parse the digest list from the path
> + */
> +static ssize_t digest_list_read(char *path, enum ops op)
> +{
> + void *data = NULL;
> + char *datap;
> + size_t size;
> + u8 actions = 0;
> + struct file *file;
> + char event_name[NAME_MAX + 9 + 1];
> + u8 digest[IMA_MAX_DIGEST_SIZE] = { 0 };
> + enum hash_algo algo;
> + int rc, pathlen = strlen(path);
> +
> + /* Remove \n. */
> + datap = path;
> + strsep(&datap, "\n");
> +
> + file = filp_open(path, O_RDONLY, 0);
> + if (IS_ERR(file)) {
> + pr_err("unable to open file: %s (%ld)", path, PTR_ERR(file));
> + return PTR_ERR(file);
> + }
> +
> + rc = kernel_read_file(file, 0, &data, INT_MAX, NULL,
> + READING_DIGEST_LIST);
> + if (rc < 0) {
> + pr_err("unable to read file: %s (%d)", path, rc);
> + goto out;
> + }
> +
> + size = rc;
> +
> + snprintf(event_name, sizeof(event_name), "%s_file_%s",
> + op == DIGEST_LIST_ADD ? "add" : "del",
> + file_dentry(file)->d_name.name);
> +
> + rc = ima_measure_critical_data("diglim", event_name, data, size, false,
> + digest, sizeof(digest));
> + if (rc < 0 && rc != -EEXIST)
> + goto out_vfree;
The digest lists could easily be measured while reading the digest list
file above in kernel_read_file(). What makes it "critical-data"? In
the SELinux case, the in memory SELinux policy is being measured and
re-measured to make sure it hasn't been modified. Is the digest list
file data being measured more than once?
I understand that with your changes to ima_measure_critical_data(),
which are now in next-integrity-testing branch, allow IMA to calculate
the file data hash.
thanks,
Mimi
> +
> + algo = ima_get_current_hash_algo();
> +
^ permalink raw reply
* [PATCH v2] KEYS: trusted: Fix trusted key backends when building as module
From: Andreas Rammhold @ 2021-07-29 18:33 UTC (permalink / raw)
To: James Bottomley, Jarkko Sakkinen, Mimi Zohar, David Howells,
James Morris, Serge E. Hallyn, Sumit Garg
Cc: Ahmad Fatoum, Andreas Rammhold, linux-integrity, keyrings,
linux-security-module, linux-kernel
Before this commit the kernel could end up with no trusted key sources
even though both of the currently supported backends (TPM and TEE) were
compiled as modules. This manifested in the trusted key type not being
registered at all.
When checking if a CONFIG_… preprocessor variable is defined we only
test for the builtin (=y) case and not the module (=m) case. By using
the IS_REACHABLE() macro we do test for both cases.
v2:
* Fixed commit message
* Switched from IS_DEFINED() to IS_REACHABLE()
Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
Signed-off-by: Andreas Rammhold <andreas@rammhold.de>
---
Here is the version that was proposed by Ahmad [1] in response to the
feedback received in the "[PATCH v2] KEYS: trusted: fix use as module
when CONFIG_TCG_TPM=m" discussion [2].
I have tested both of the patches on v5.13 and they both fix the problem
I originally encountered.
[1] https://lore.kernel.org/keyrings/fe39a449-88df-766b-a13a-290f4847d43e@pengutronix.de/
[2] https://lore.kernel.org/keyrings/20210721160258.7024-1-a.fatoum@pengutronix.de/
security/keys/trusted-keys/trusted_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index d5c891d8d353..5b35f1b87644 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -27,10 +27,10 @@ module_param_named(source, trusted_key_source, charp, 0);
MODULE_PARM_DESC(source, "Select trusted keys source (tpm or tee)");
static const struct trusted_key_source trusted_key_sources[] = {
-#if defined(CONFIG_TCG_TPM)
+#if IS_REACHABLE(CONFIG_TCG_TPM)
{ "tpm", &trusted_key_tpm_ops },
#endif
-#if defined(CONFIG_TEE)
+#if IS_REACHABLE(CONFIG_TEE)
{ "tee", &trusted_key_tee_ops },
#endif
};
--
2.32.0
^ permalink raw reply related
* Re: [RFC PATCH v1] fscrypt: support encrypted and trusted keys
From: Eric Biggers @ 2021-07-29 18:28 UTC (permalink / raw)
To: Ahmad Fatoum
Cc: Sumit Garg, Theodore Y. Ts'o, Jaegeuk Kim, Jarkko Sakkinen,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
David Howells, linux-fscrypt,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
open list:SECURITY SUBSYSTEM, open list:ASYMMETRIC KEYS,
Linux Kernel Mailing List, git, Omar Sandoval,
Pengutronix Kernel Team
In-Reply-To: <1530428a-ad2c-a169-86a7-24bfafb9b9bd@pengutronix.de>
On Thu, Jul 29, 2021 at 11:07:00AM +0200, Ahmad Fatoum wrote:
> >> Most people don't change defaults.
> >>
> >> Essentially your same argument was used for Dual_EC_DRBG; people argued it was
> >> okay to standardize because people had the option to choose their own constants
> >> if they felt the default constants were backdoored. That didn't really matter,
> >> though, since in practice everyone just used the default constants.
>
> I'd appreciate your feedback on my CAAM series if you think the defaults
> can be improved. Trusted keys are no longer restricted to TPMs,
> so users of other backends shouldn't be dismissed, because one backend
> can be used with fscrypt by alternative means.
I already gave feedback:
https://lkml.kernel.org/keyrings/YGOcZtkw3ZM5kvl6@gmail.com
https://lkml.kernel.org/keyrings/YGUHBelwhvJDhKoo@gmail.com
https://lkml.kernel.org/keyrings/YGViOc3DG+Pjuur6@sol.localdomain
> >>>> - trusted and encrypted keys aren't restricted to specific uses in the kernel
> >>>> (like the fscrypt-provisioning key type is) but rather are general-purpose.
> >>>> Hence, it may be possible to leak their contents to userspace by requesting
> >>>> their use for certain algorithms/features, e.g. to encrypt a dm-crypt target
> >>>> using a weak cipher that is vulnerable to key recovery attacks.
> >>>
> >>> The footgun is already there by allowing users to specify their own
> >>>
> >>> raw key. Users can already use $keyid for dm-crypt and then do
> >>>
> >>> $ keyctl pipe $keyid | fscryptctl add_key /mnt
> >>>
> >>> The responsibility to not reuse key material already lies with the users,
> >>> regardless if they handle the raw key material directly or indirectly via
> >>> a trusted key description/ID.
> >>
> >> Elsewhere you are claiming that "trusted" keys can never be disclosed to
> >> userspace. So you can't rely on userspace cooperating, right?
>
> The users I meant are humans, e.g. system integrators. They need to think about
>
> burning fuses, signing bootloaders, verifying kernel and root file systems,
>
> encrypting file systems and safekeeping their crypto keys. Ample opportunity for
>
> stuff to go wrong. They would benefit from having relevant kernel functionality
>
> integrate with each other instead of having to carry downstream patches, which
> we and many others[1] did for years. We now finally have a chance to drop this
> technical debt thanks to Sumit's trusted key rework and improve user security
> along the way.
>
> So, Eric, how should we proceed?
>
It is probably inevitable that this be added, but you need to document it
properly and not make misleading claims like "The key material is generated
within the kernel" (for the TPM "trust" source the default is to use the TPM's
RNG, *not* the kernel RNG), and "is never disclosed to userspace in clear text"
(that's only guaranteed to be true for non-malicious userspace). Also please
properly document that this is mainly intended for accessing key wrapping
hardware such as CAAM that can't be accessed from userspace in another way like
TPMs can.
- Eric
^ permalink raw reply
* [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-07-29 16:19 UTC (permalink / raw)
To: jarkko
Cc: peterhuewe, jgg, linux-integrity, linux-security-module,
linux-kernel, Stefan Berger, Nayna Jain, George Wilson,
Nageswara R Sastry
From: Stefan Berger <stefanb@linux.ibm.com>
When rngd is run as root then lots of these types of message will appear
in the kernel log if the TPM has been configure to provide random bytes:
[ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
The issue is caused by the following call that is interrupted while
waiting for the TPM's response.
sig = wait_event_interruptible(ibmvtpm->wq,
!ibmvtpm->tpm_processing_cmd);
Rather than waiting for the response in the low level driver, have it use
the polling loop in tpm_try_transmit() that uses a command's duration to
poll until a result has been returned by the TPM, thus ending when the
timeout has occurred but not responding to signals and ctrl-c anymore.
To stay in this polling loop we now extend tpm_ibmvtpm_status() to return
PM_STATUS_BUSY for as long as the vTPM is busy. Since we will need the
timeouts in this loop now we get the TPM 1.2 and TPM 2 timeouts with
tpm_get_timeouts().
Rename the tpm_processing_cmd to tpm_status in ibmvtpm_dev and set the
TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
To recreat the issue start rngd like this:
sudo rngd -r /dev/hwrng -t
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1981473
Fixes: 6674ff145eef ("tpm_ibmvtpm: properly handle interrupted packet receptions")
Cc: Nayna Jain <nayna@linux.ibm.com>
Cc: George Wilson <gcwilson@linux.ibm.com>
Reported-by: Nageswara R Sastry <rnsastry@linux.ibm.com>
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>
---
drivers/char/tpm/tpm_ibmvtpm.c | 31 ++++++++++++++++++-------------
drivers/char/tpm/tpm_ibmvtpm.h | 3 ++-
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/char/tpm/tpm_ibmvtpm.c b/drivers/char/tpm/tpm_ibmvtpm.c
index 903604769de9..5d795866b483 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.c
+++ b/drivers/char/tpm/tpm_ibmvtpm.c
@@ -106,17 +106,12 @@ static int tpm_ibmvtpm_recv(struct tpm_chip *chip, u8 *buf, size_t count)
{
struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
u16 len;
- int sig;
if (!ibmvtpm->rtce_buf) {
dev_err(ibmvtpm->dev, "ibmvtpm device is not ready\n");
return 0;
}
- sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
- if (sig)
- return -EINTR;
-
len = ibmvtpm->res_len;
if (count < len) {
@@ -220,11 +215,12 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
return -EIO;
}
- if (ibmvtpm->tpm_processing_cmd) {
+ if ((ibmvtpm->tpm_status & TPM_STATUS_BUSY)) {
dev_info(ibmvtpm->dev,
"Need to wait for TPM to finish\n");
/* wait for previous command to finish */
- sig = wait_event_interruptible(ibmvtpm->wq, !ibmvtpm->tpm_processing_cmd);
+ sig = wait_event_interruptible(ibmvtpm->wq,
+ (ibmvtpm->tpm_status & TPM_STATUS_BUSY) == 0);
if (sig)
return -EINTR;
}
@@ -237,7 +233,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
* set the processing flag before the Hcall, since we may get the
* result (interrupt) before even being able to check rc.
*/
- ibmvtpm->tpm_processing_cmd = true;
+ ibmvtpm->tpm_status |= TPM_STATUS_BUSY;
again:
rc = ibmvtpm_send_crq(ibmvtpm->vdev,
@@ -255,7 +251,7 @@ static int tpm_ibmvtpm_send(struct tpm_chip *chip, u8 *buf, size_t count)
goto again;
}
dev_err(ibmvtpm->dev, "tpm_ibmvtpm_send failed rc=%d\n", rc);
- ibmvtpm->tpm_processing_cmd = false;
+ ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
}
spin_unlock(&ibmvtpm->rtce_lock);
@@ -269,7 +265,9 @@ static void tpm_ibmvtpm_cancel(struct tpm_chip *chip)
static u8 tpm_ibmvtpm_status(struct tpm_chip *chip)
{
- return 0;
+ struct ibmvtpm_dev *ibmvtpm = dev_get_drvdata(&chip->dev);
+
+ return ibmvtpm->tpm_status;
}
/**
@@ -457,7 +455,7 @@ static const struct tpm_class_ops tpm_ibmvtpm = {
.send = tpm_ibmvtpm_send,
.cancel = tpm_ibmvtpm_cancel,
.status = tpm_ibmvtpm_status,
- .req_complete_mask = 0,
+ .req_complete_mask = TPM_STATUS_BUSY,
.req_complete_val = 0,
.req_canceled = tpm_ibmvtpm_req_canceled,
};
@@ -550,7 +548,7 @@ static void ibmvtpm_crq_process(struct ibmvtpm_crq *crq,
case VTPM_TPM_COMMAND_RES:
/* len of the data in rtce buffer */
ibmvtpm->res_len = be16_to_cpu(crq->len);
- ibmvtpm->tpm_processing_cmd = false;
+ ibmvtpm->tpm_status &= ~TPM_STATUS_BUSY;
wake_up_interruptible(&ibmvtpm->wq);
return;
default:
@@ -688,8 +686,15 @@ static int tpm_ibmvtpm_probe(struct vio_dev *vio_dev,
goto init_irq_cleanup;
}
- if (!strcmp(id->compat, "IBM,vtpm20")) {
+
+ if (!strcmp(id->compat, "IBM,vtpm20"))
chip->flags |= TPM_CHIP_FLAG_TPM2;
+
+ rc = tpm_get_timeouts(chip);
+ if (rc)
+ goto init_irq_cleanup;
+
+ if (chip->flags & TPM_CHIP_FLAG_TPM2) {
rc = tpm2_get_cc_attrs_tbl(chip);
if (rc)
goto init_irq_cleanup;
diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
index b92aa7d3e93e..252f1cccdfc5 100644
--- a/drivers/char/tpm/tpm_ibmvtpm.h
+++ b/drivers/char/tpm/tpm_ibmvtpm.h
@@ -41,7 +41,8 @@ struct ibmvtpm_dev {
wait_queue_head_t wq;
u16 res_len;
u32 vtpm_version;
- bool tpm_processing_cmd;
+ u8 tpm_status;
+#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
};
#define CRQ_RES_BUF_SIZE PAGE_SIZE
--
2.31.1
^ permalink raw reply related
* Re: [PATCH v2 1/4] landlock.7: Add a new page to introduce Landlock
From: Alejandro Colomar (man-pages) @ 2021-07-29 14:56 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Jann Horn, Jonathan Corbet, Kees Cook, Randy Dunlap,
Vincent Dagonneau, landlock, linux-kernel, linux-man,
linux-security-module, Mickaël Salaün, Michael Kerrisk
In-Reply-To: <20210712155745.831580-2-mic@digikod.net>
Hi Mickaël,
On 7/12/21 5:57 PM, Mickaël Salaün wrote:
> From: Mickaël Salaün <mic@linux.microsoft.com>
>
> From the user point of view, Landlock is a set of system calls enabling
> to build and enforce a set of access-control rules. A ruleset can be
> created with landlock_create_ruleset(2), populated with
> landlock_add_rule(2) and enforced with landlock_restrict_self(2). This
> man page gives an overview of the whole mechanism. Details of these
> system calls are documented in their respective man pages.
>
> This is an adaptation of
> https://www.kernel.org/doc/html/v5.13/userspace-api/landlock.html
>
> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
> Link: https://lore.kernel.org/r/20210712155745.831580-2-mic@digikod.net
Please see some comments below, mostly about formatting.
The text looks good to me.
Thanks,
Alex
> ---
>
> Changes since v1:
> * Replace all ".I" with ".IR", except when used for titles.
Sorry, but I actually prefer the opposite: Use .I unless you really need .IR
If there was a misunderstanding about this, I'm sorry.
> * Append punctuation to ".IR" and ".BR" when it makes sense (requested
> by Alejandro Colomar).
> * Cut lines according to the semantic newline rules (requested by
> Alejandro Colomar).
> * Remove roman style from ".TP" section titles (requested by Alejandro
> Colomar).
> * Add comma after "i.e." and "e.g.".
> * Move the example in a new EXAMPLES section.
> * Improve title.
> * Explain the LSM acronym at first use.
> ---
> man7/landlock.7 | 356 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 356 insertions(+)
> create mode 100644 man7/landlock.7
>
> diff --git a/man7/landlock.7 b/man7/landlock.7
> new file mode 100644
> index 000000000000..c89f5b1cabb6
> --- /dev/null
> +++ b/man7/landlock.7
> @@ -0,0 +1,356 @@
> +.\" Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
> +.\" Copyright © 2019-2020 ANSSI
> +.\" Copyright © 2021 Microsoft Corporation
> +.\"
> +.\" %%%LICENSE_START(VERBATIM)
> +.\" Permission is granted to make and distribute verbatim copies of this
> +.\" manual provided the copyright notice and this permission notice are
> +.\" preserved on all copies.
> +.\"
> +.\" Permission is granted to copy and distribute modified versions of this
> +.\" manual under the conditions for verbatim copying, provided that the
> +.\" entire resulting derived work is distributed under the terms of a
> +.\" permission notice identical to this one.
> +.\"
> +.\" Since the Linux kernel and libraries are constantly changing, this
> +.\" manual page may be incorrect or out-of-date. The author(s) assume no
> +.\" responsibility for errors or omissions, or for damages resulting from
> +.\" the use of the information contained herein. The author(s) may not
> +.\" have taken the same level of care in the production of this manual,
> +.\" which is licensed free of charge, as they might when working
> +.\" professionally.
> +.\"
> +.\" Formatted or processed versions of this manual, if unaccompanied by
> +.\" the source, must acknowledge the copyright and authors of this work.
> +.\" %%%LICENSE_END
> +.\"
> +.TH LANDLOCK 7 2021-06-27 Linux "Linux Programmer's Manual"
> +.SH NAME
> +Landlock \- unprivileged access-control
> +.SH DESCRIPTION
> +Landlock is an access-control system that enables any processes to // securely /J/
I'll add some line breaks [//] and line joins [/J/] through the email.
> +restrict themselves and their future children.
> +Because Landlock is a stackable Linux Security Module (LSM),
> +it makes possible to create safe security sandboxes as new security layers
suggested wfix: "it makes it possible" or "it is possible"?
> +in addition to the existing system-wide access-controls.
> +This kind of sandbox is expected to help mitigate // the security impact of /J/ > +bugs, // and unexpected or malicious behaviors in applications.
See line-break fixes above.
> +.PP
> +A Landlock security policy is a set of access rights
> +(e.g., open a file in read-only, make a directory, etc.)
> +tied to a file hierarchy.
> +Such policy can be configured and enforced by processes for themselves
> +using three system calls:
> +.IP \(bu 2
> +.BR landlock_create_ruleset (2)
> +creates a new ruleset;
> +.IP \(bu
> +.BR landlock_add_rule (2)
> +adds a new rule to a ruleset;
> +.IP \(bu
> +.BR landlock_restrict_self (2)
> +enforces a ruleset on the calling thread.
> +.PP
> +To be able to use these system calls,
> +the running kernel must support Landlock and // it must be enabled at boot /J/
> +time.
See line-break fixes above
> +.\"
> +.SS Landlock rules
> +A Landlock rule describes an action on an object.
> +An object is currently a file hierarchy,
> +and the related filesystem actions are defined with access rights (see
> +.BR landlock_add_rule (2)).
> +A set of rules is aggregated in a ruleset, // which can /J/
> +then restrict the thread enforcing it, // and its future children.
See line-break fixes above.
> +.\"
> +.SS Filesystem actions
> +These flags enable to restrict a sandboxed process to a // set of actions on /J/
> +files and directories. > +Files or directories opened before the sandboxing // are not subject
to these /J/
> +restrictions.
See line-break fixes above.
> +See
> +.BR landlock_add_rule (2)
> +and
> +.BR landlock_create_ruleset (2)
> +for more context.
> +.PP
> +A file can only receive these access rights:
> +.TP
> +.B LANDLOCK_ACCESS_FS_EXECUTE
> +Execute a file.
> +.TP
> +.B LANDLOCK_ACCESS_FS_WRITE_FILE
> +Open a file with write access.
> +.TP
> +.B LANDLOCK_ACCESS_FS_READ_FILE
> +Open a file with read access.
> +.PP
> +A directory can receive access rights related to files or directories.
> +The following access right is applied to the directory itself,
> +and the directories beneath it:
> +.TP
> +.B LANDLOCK_ACCESS_FS_READ_DIR
> +Open a directory or list its content.
> +.PP
> +However,
> +the following access rights only apply to the content of a directory,
> +not the directory itself:
> +.TP
> +.B LANDLOCK_ACCESS_FS_REMOVE_DIR
> +Remove an empty directory or rename one.
> +.TP
> +.B LANDLOCK_ACCESS_FS_REMOVE_FILE
> +Unlink (or rename) a file.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_CHAR
> +Create (or rename or link) a character device.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_DIR
> +Create (or rename) a directory.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_REG
> +Create (or rename or link) a regular file.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_SOCK
> +Create (or rename or link) a UNIX domain socket.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_FIFO
> +Create (or rename or link) a named pipe.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_BLOCK
> +Create (or rename or link) a block device.
> +.TP
> +.B LANDLOCK_ACCESS_FS_MAKE_SYM
> +Create (or rename or link) a symbolic link.
> +.\"
> +.SS Layers of file path access rights
> +Each time a thread enforces a ruleset on itself, // it updates its Landlock /J/
See line-break fixes above
> +domain with a new layer of policy.
> +Indeed, this complementary policy is composed with the potentially other
> +rulesets already restricting this thread.
> +A sandboxed thread can then safely add more constraints to itself with a
> +new enforced ruleset.
> +.PP
> +One policy layer grants access to a file path // if at least one of its rules /J/
> +encountered on the path grants the access.
> +A sandboxed thread can only access a file path // if all its enforced policy /J/
> +layers grant the access // as well as all the other system access controls
> +(e.g., filesystem DAC, other LSM policies, etc.).
See line-break fixes above.
> +.\"
> +.SS Bind mounts and OverlayFS
> +Landlock enables restricting access to file hierarchies,
> +which means that these access rights can be propagated with bind mounts
> +(cf.
> +.BR mount_namespaces (7))
> +but not with OverlayFS.
> +.PP
> +A bind mount mirrors a source file hierarchy to a destination.
> +The destination hierarchy is then composed of the exact same files,
> +on which Landlock rules can be tied, // either via the source or the /J/
> +destination path.
> +These rules restrict access when they are encountered on a path,
> +which means that they can restrict access to // multiple file hierarchies at /J/
> +the same time,
> +whether these hierarchies are the result of bind mounts or not.
See line-break fixes above.
> +.PP
> +An OverlayFS mount point consists of upper and lower layers.
> +These layers are combined in a merge directory, result of the mount point.
> +This merge hierarchy may include files from the upper and lower layers,
> +but modifications performed on the merge hierarchy // only reflects on the /J/
s/reflects/reflect/
> +upper layer.
> +From a Landlock policy point of view,
> +each OverlayFS layers and merge hierarchies are standalone and contains
> +their own set of files and directories,
> +which is different from bind mounts.
Incorrect mix of singular and plural, I think.
> +A policy restricting an OverlayFS layer will not restrict the resulted
> +merged hierarchy, and vice versa.
> +Landlock users should then only think about file hierarchies they want to
> +allow access to, regardless of the underlying filesystem.
> +.\"
> +.SS Inheritance
> +Every new thread resulting from a
> +.BR clone (2)
> +inherits Landlock domain restrictions from its parent.
> +This is similar to the
> +.BR seccomp (2)
> +inheritance or any other LSM dealing with task's
Not sure:
s/task/a task/
or
s/task's/tasks'/
> +.BR credentials (7).
> +For instance, one process's thread may apply Landlock rules to itself,
s/process's/process'/
> +but they will not be automatically applied to other sibling threads
> +(unlike POSIX thread credential changes, cf.
> +.BR nptl (7)).
> +.PP
> +When a thread sandboxes itself, // we have the guarantee that the related /J/
> +security policy // will stay enforced on all this thread's descendants.
> +This allows creating standalone and modular security policies // per /J/
> +application,
> +which will automatically be composed between themselves // according to their /J/
> +runtime parent policies.
> +.\"
> +.SS Ptrace restrictions
> +A sandboxed process has less privileges than a non-sandboxed process and
> +must then be subject to additional restrictions // when manipulating another /J/
> +process.
> +To be allowed to use
> +.BR ptrace (2)
> +and related syscalls on a target process,
> +a sandboxed process should have a subset of the target process rules,
> +which means the tracee must be in a sub-domain of the tracer.
> +.SH VERSIONS
> +Landlock was added in Linux 5.13.
> +.SH NOTES
> +Landlock is enabled by CONFIG_SECURITY_LANDLOCK.
.BR CONFIG_SECURITY_LANDLOCK .
> +The
> +.IR lsm=lsm1,...,lsmN
s/.IR/.I/
> +command line parameter controls the sequence of the initialization of
> +Linux Security Modules.
> +It must contain the string
> +.IR landlock
s/.IR/.I
> +to enable Landlock.
> +If the command line parameter is not specified,
> +the initialization falls back to the value of the deprecated
> +.IR security=
s/.IR/.I/
> +command line parameter and further to the value of CONFIG_LSM.
> +We can check that Landlock is enabled by looking for
> +.IR "landlock: Up and running."
s/.IR/.I/
> +in kernel logs.
> +.PP
> +It is currently not possible to restrict some file-related actions
> +accessible through these syscall families:
When using syscall to refer to system call (not the function syscall(2)),
we use the extended form "system call".
> +.BR chdir (2),
> +.BR truncate (2),
> +.BR stat (2),
> +.BR flock (2),
> +.BR chmod (2),
> +.BR chown (2),
> +.BR setxattr (2),
> +.BR utime (2),
> +.BR ioctl (2),
> +.BR fcntl (2),
> +.BR access (2).
> +Future Landlock evolutions will enable to restrict them.
> +.SH EXAMPLES
I'd prefer a complete example with a main function, if you can come up
with such one. If not, this will be ok.
> +We first need to create the ruleset that will contain our rules.
> +For this example,
> +the ruleset will contain rules that only allow read actions,
> +but write actions will be denied.
> +The ruleset then needs to handle both of these kind of actions.
> +See below for the description of filesystem actions.
> +.PP
> +.in +4n
> +.EX
> +int ruleset_fd;
> +struct landlock_ruleset_attr ruleset_attr = {
> + .handled_access_fs =
> + LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_WRITE_FILE |
> + LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_READ_DIR |
> + LANDLOCK_ACCESS_FS_REMOVE_DIR |
> + LANDLOCK_ACCESS_FS_REMOVE_FILE |
> + LANDLOCK_ACCESS_FS_MAKE_CHAR |
> + LANDLOCK_ACCESS_FS_MAKE_DIR |
> + LANDLOCK_ACCESS_FS_MAKE_REG |
> + LANDLOCK_ACCESS_FS_MAKE_SOCK |
> + LANDLOCK_ACCESS_FS_MAKE_FIFO |
> + LANDLOCK_ACCESS_FS_MAKE_BLOCK |
> + LANDLOCK_ACCESS_FS_MAKE_SYM,
> +};
> +
> +ruleset_fd = landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> +if (ruleset_fd < 0) {
> + perror("Failed to create a ruleset");
> + return 1;
> +}
> +.EE
> +.in
> +.PP
> +We can now add a new rule to this ruleset thanks to the returned file
> +descriptor referring to this ruleset.
> +The rule will only allow reading the file hierarchy
> +.IR /usr .
> +Without another rule, write actions would then be denied by the ruleset.
> +To add
> +.IR /usr
> +to the ruleset, we open it with the
> +.IR O_PATH
> +flag and fill the
> +.IR "struct landlock_path_beneath_attr"
> +with this file descriptor.
> +.PP
> +.in +4n
> +.EX
> +int err;
> +struct landlock_path_beneath_attr path_beneath = {
> + .allowed_access =
> + LANDLOCK_ACCESS_FS_EXECUTE |
> + LANDLOCK_ACCESS_FS_READ_FILE |
> + LANDLOCK_ACCESS_FS_READ_DIR,
> +};
> +
> +path_beneath.parent_fd = open("/usr", O_PATH | O_CLOEXEC);
> +if (path_beneath.parent_fd < 0) {
> + perror("Failed to open file");
> + close(ruleset_fd);
> + return 1;
> +}
> +err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
> + &path_beneath, 0);
> +close(path_beneath.parent_fd);
> +if (err) {
> + perror("Failed to update ruleset");
> + close(ruleset_fd);
> + return 1;
> +}
> +.EE
> +.in
> +.PP
> +We now have a ruleset with one rule allowing read access to
> +.IR /usr
> +while denying all other handled accesses for the filesystem.
> +The next step is to restrict the current thread from gaining more
> +privileges
> +(e.g., thanks to a set-user-ID binary).
> +.PP
> +.in +4n
> +.EX
> +if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)) {
> + perror("Failed to restrict privileges");
> + close(ruleset_fd);
> + return 1;
> +}
> +.EE
> +.in
> +.PP
> +The current thread is now ready to sandbox itself with the ruleset.
> +.PP
> +.in +4n
> +.EX
> +if (landlock_restrict_self(ruleset_fd, 0)) {
> + perror("Failed to enforce ruleset");
> + close(ruleset_fd);
> + return 1;
> +}
> +close(ruleset_fd);
> +.EE
> +.in
> +.PP
> +If the
> +.BR landlock_restrict_self (2)
> +system call succeeds, the current thread is now restricted and this policy
> +will be enforced on all its subsequently created children as well.
> +Once a thread is landlocked, there is no way to remove its security policy;
> +only adding more restrictions is allowed.
> +These threads are now in a new Landlock domain, // merge of their parent one /J/
> +(if any) with the new ruleset.
> +.PP
> +Full working code can be found in
> +.UR https://git.kernel.org\:/pub\:/scm\:/linux\:/kernel\:/git\:/stable\:/linux.git\:/tree\:/samples\:/landlock\:/sandboxer.c
> +.UE
> +.SH SEE ALSO
> +.BR landlock_create_ruleset (2),
> +.BR landlock_add_rule (2),
> +.BR landlock_restrict_self (2)
> +.PP
> +.UR https://landlock.io\:/
> +.UE
>
--
Alejandro Colomar
Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/
http://www.alejandro-colomar.es/
^ permalink raw reply
* Re: [PATCH] tpm: ibmvtpm: Avoid error message when process gets signal while waiting
From: Stefan Berger @ 2021-07-29 13:39 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Stefan Berger, peterhuewe, jgg, linux-integrity,
linux-security-module, linux-kernel, Nayna Jain, George Wilson,
Nageswara R Sastry
In-Reply-To: <20210728215033.dhnekvksekalhcrn@kernel.org>
On 7/28/21 5:50 PM, Jarkko Sakkinen wrote:
> On Mon, Jul 26, 2021 at 11:00:51PM -0400, Stefan Berger wrote:
>> On 7/26/21 10:42 PM, Jarkko Sakkinen wrote:
>>> On Mon, Jul 12, 2021 at 12:25:05PM -0400, Stefan Berger wrote:
>>>> From: Stefan Berger <stefanb@linux.ibm.com>
>>>>
>>>> When rngd is run as root then lots of these types of message will appear
>>>> in the kernel log if the TPM has been configure to provide random bytes:
>>>>
>>>> [ 7406.275163] tpm tpm0: tpm_transmit: tpm_recv: error -4
>>>>
>>>> The issue is caused by the following call that is interrupted while
>>>> waiting for the TPM's response.
>>>>
>>>> sig = wait_event_interruptible(ibmvtpm->wq,
>>>> !ibmvtpm->tpm_processing_cmd);
>>>>
>>>> The solution is to use wait_event() instead.
>>> Why?
>> So it becomes uninterruptible and these error messages go away.
> We do not want to make a process uninterruptible. That would prevent
> killing it.
I guess we'll have to go back to this one then:
https://www.spinics.net/lists/linux-integrity/msg16741.html
Stefan
>
> /Jarkko
^ permalink raw reply
* Re: [RFC PATCH v1] fscrypt: support encrypted and trusted keys
From: Ahmad Fatoum @ 2021-07-29 9:07 UTC (permalink / raw)
To: Sumit Garg, Eric Biggers
Cc: Theodore Y. Ts'o, Jaegeuk Kim, Jarkko Sakkinen, James Morris,
Serge E. Hallyn, James Bottomley, Mimi Zohar, David Howells,
linux-fscrypt, open list:HARDWARE RANDOM NUMBER GENERATOR CORE,
linux-integrity, open list:SECURITY SUBSYSTEM,
open list:ASYMMETRIC KEYS, Linux Kernel Mailing List, git,
Omar Sandoval, Pengutronix Kernel Team
In-Reply-To: <CAFA6WYO-h+ngCAT_PS=bZTQkBBtOpBRUmZNP4zhvRuLDJYQXkA@mail.gmail.com>
On 29.07.21 07:56, Sumit Garg wrote:
> Hi Eric,
>
> On Wed, 28 Jul 2021 at 21:35, Eric Biggers <ebiggers@kernel.org> wrote:
>>
>> On Wed, Jul 28, 2021 at 10:50:42AM +0200, Ahmad Fatoum wrote:
>>> Hello Eric,
>>>
>>> On 27.07.21 18:38, Eric Biggers wrote:
>>>> On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote:
>>>>> For both v1 and v2 key setup mechanisms, userspace supplies the raw key
>>>>> material to the kernel after which it is never again disclosed to
>>>>> userspace.
>>>>>
>>>>> Use of encrypted and trusted keys offers stronger guarantees:
>>>>> The key material is generated within the kernel and is never disclosed to
>>>>> userspace in clear text and, in the case of trusted keys, can be
>>>>> directly rooted to a trust source like a TPM chip.
>>>>
>>>> Please include a proper justification for this feature
>>>
>>> I've patches pending for extending trusted keys to wrap the key sealing
>>> functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to
>>> generate key material in the factory, have the CAAM encrypt it using its
>>> undisclosed unique key and pass it to userspace as encrypted blob that is
>>> persisted to an unencrypted volume. The intention is to thwart offline
>>> decryption of an encrypted file system in an embedded system, where a
>>> passphrase can't be supplied by an end user.
>>>
>>> Employing TPM and TEE trusted keys with this is already possible with
>>> dm-crypt, but I'd like this to be possible out-of-the-box with
>>> ubifs + fscrypt as well.
>>
>> Why not do the key management in userspace, like tpm-tools
>> (https://github.com/tpm2-software/tpm2-tools)? There are a lot of uses for this
>> type of hardware besides in-kernel crypto. See
>> https://wiki.archlinux.org/title/Trusted_Platform_Module for all the things you
>> can do with the TPM on Linux, including LUKS encryption; this is all with
>> userspace key management. Wouldn't the CAAM hardware be useful for similar
>> purposes and thus need a similar design as well, e.g. with functionality exposed
>> through some /dev node for userspace to use? Or are you saying it will only
>> ever be useful for in-kernel crypto?
>
> AFAIK from my prior experience while working with CAAM engine during
> my time at NXP, it is generally a crypto engine with additional
> security properties like one discussed here to protect keys (blob
> encap and decap) etc. But it doesn't offer user authentication similar
> to what a TPM (ownership) can offer. Although, one should be able to
> expose CAAM via /dev node but I am not sure if that would be really
> useful without user authentication. I think similar should be the case
> for other crypto engines with additional security properties.
>
> With restriction of CAAM's security properties to kernel crypto we
> could at least ensure a kernel boundary that should offer enough
> resistance from malicious user space attacks.
Which are a real possibility given the CAAM's direct memory access.
We now have safe interfaces mainline wrapping it for HWRNG, crypto
acceleration and hopefully soon key sealing and I don't think throwing
existing kernel driver support away and inventing a new CAAM uAPI is a
practical alternative.
>>>> Note that there are several design flaws with the encrypted and trusted key
>>>> types:
>>>>
>>>> - By default, trusted keys are generated using the TPM's RNG rather than the
>>>> kernel's RNG, which places all trust in an unauditable black box.
>>>
>
> With regards to trusted keys generated using the TEE's RNG, the
> underlying implementation being OP-TEE [1] which is an open source TEE
> implementation built on top of Arm TrustZone providing the hardware
> based isolation among the TEE and Linux. So regarding auditability, it
> should be comparatively easier to audit the TEE components designed
> with a goal of minimal footprint when compared with Linux kernel.
>
> [1] https://github.com/OP-TEE/optee_os
>
>>> Patch to fix that awaits feedback on linux-integrity[2].
>>
>> It does *not* fix it, as your patch only provides an option to use the kernel's
>> RNG whereas the default is still the TPM's RNG.
>>
>
> Yes in case of TPM, default is still TPM's RNG but with Ahmad's patch
> #2, the trust source backend like CAAM should be able to use kernel's
> RNG by default.
Exactly.
>> Most people don't change defaults.
>>
>> Essentially your same argument was used for Dual_EC_DRBG; people argued it was
>> okay to standardize because people had the option to choose their own constants
>> if they felt the default constants were backdoored. That didn't really matter,
>> though, since in practice everyone just used the default constants.
I'd appreciate your feedback on my CAAM series if you think the defaults
can be improved. Trusted keys are no longer restricted to TPMs,
so users of other backends shouldn't be dismissed, because one backend
can be used with fscrypt by alternative means.
>>>> - trusted and encrypted keys aren't restricted to specific uses in the kernel
>>>> (like the fscrypt-provisioning key type is) but rather are general-purpose.
>>>> Hence, it may be possible to leak their contents to userspace by requesting
>>>> their use for certain algorithms/features, e.g. to encrypt a dm-crypt target
>>>> using a weak cipher that is vulnerable to key recovery attacks.
>>>
>>> The footgun is already there by allowing users to specify their own
>>>
>>> raw key. Users can already use $keyid for dm-crypt and then do
>>>
>>> $ keyctl pipe $keyid | fscryptctl add_key /mnt
>>>
>>> The responsibility to not reuse key material already lies with the users,
>>> regardless if they handle the raw key material directly or indirectly via
>>> a trusted key description/ID.
>>
>> Elsewhere you are claiming that "trusted" keys can never be disclosed to
>> userspace. So you can't rely on userspace cooperating, right?
The users I meant are humans, e.g. system integrators. They need to think about
burning fuses, signing bootloaders, verifying kernel and root file systems,
encrypting file systems and safekeeping their crypto keys. Ample opportunity for
stuff to go wrong. They would benefit from having relevant kernel functionality
integrate with each other instead of having to carry downstream patches, which
we and many others[1] did for years. We now finally have a chance to drop this
technical debt thanks to Sumit's trusted key rework and improve user security
along the way.
So, Eric, how should we proceed?
[1]: See the CAAM series cover letter for a history of CAAM sealing upstreaming
efforts
Cheers,
Ahmad
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [RFC PATCH v1] fscrypt: support encrypted and trusted keys
From: Sumit Garg @ 2021-07-29 5:56 UTC (permalink / raw)
To: Eric Biggers
Cc: Ahmad Fatoum, Theodore Y. Ts'o, Jaegeuk Kim, Jarkko Sakkinen,
James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
David Howells, linux-fscrypt,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, linux-integrity,
open list:SECURITY SUBSYSTEM, open list:ASYMMETRIC KEYS,
Linux Kernel Mailing List, git, Omar Sandoval,
Pengutronix Kernel Team
In-Reply-To: <YQGAOTdQRHFv9rlr@gmail.com>
Hi Eric,
On Wed, 28 Jul 2021 at 21:35, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Wed, Jul 28, 2021 at 10:50:42AM +0200, Ahmad Fatoum wrote:
> > Hello Eric,
> >
> > On 27.07.21 18:38, Eric Biggers wrote:
> > > On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote:
> > >> For both v1 and v2 key setup mechanisms, userspace supplies the raw key
> > >> material to the kernel after which it is never again disclosed to
> > >> userspace.
> > >>
> > >> Use of encrypted and trusted keys offers stronger guarantees:
> > >> The key material is generated within the kernel and is never disclosed to
> > >> userspace in clear text and, in the case of trusted keys, can be
> > >> directly rooted to a trust source like a TPM chip.
> > >
> > > Please include a proper justification for this feature
> >
> > I've patches pending for extending trusted keys to wrap the key sealing
> > functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to
> > generate key material in the factory, have the CAAM encrypt it using its
> > undisclosed unique key and pass it to userspace as encrypted blob that is
> > persisted to an unencrypted volume. The intention is to thwart offline
> > decryption of an encrypted file system in an embedded system, where a
> > passphrase can't be supplied by an end user.
> >
> > Employing TPM and TEE trusted keys with this is already possible with
> > dm-crypt, but I'd like this to be possible out-of-the-box with
> > ubifs + fscrypt as well.
>
> Why not do the key management in userspace, like tpm-tools
> (https://github.com/tpm2-software/tpm2-tools)? There are a lot of uses for this
> type of hardware besides in-kernel crypto. See
> https://wiki.archlinux.org/title/Trusted_Platform_Module for all the things you
> can do with the TPM on Linux, including LUKS encryption; this is all with
> userspace key management. Wouldn't the CAAM hardware be useful for similar
> purposes and thus need a similar design as well, e.g. with functionality exposed
> through some /dev node for userspace to use? Or are you saying it will only
> ever be useful for in-kernel crypto?
AFAIK from my prior experience while working with CAAM engine during
my time at NXP, it is generally a crypto engine with additional
security properties like one discussed here to protect keys (blob
encap and decap) etc. But it doesn't offer user authentication similar
to what a TPM (ownership) can offer. Although, one should be able to
expose CAAM via /dev node but I am not sure if that would be really
useful without user authentication. I think similar should be the case
for other crypto engines with additional security properties.
With restriction of CAAM's security properties to kernel crypto we
could at least ensure a kernel boundary that should offer enough
resistance from malicious user space attacks.
>
> > > Note that there are several design flaws with the encrypted and trusted key
> > > types:
> > >
> > > - By default, trusted keys are generated using the TPM's RNG rather than the
> > > kernel's RNG, which places all trust in an unauditable black box.
> >
With regards to trusted keys generated using the TEE's RNG, the
underlying implementation being OP-TEE [1] which is an open source TEE
implementation built on top of Arm TrustZone providing the hardware
based isolation among the TEE and Linux. So regarding auditability, it
should be comparatively easier to audit the TEE components designed
with a goal of minimal footprint when compared with Linux kernel.
[1] https://github.com/OP-TEE/optee_os
> > Patch to fix that awaits feedback on linux-integrity[2].
>
> It does *not* fix it, as your patch only provides an option to use the kernel's
> RNG whereas the default is still the TPM's RNG.
>
Yes in case of TPM, default is still TPM's RNG but with Ahmad's patch
#2, the trust source backend like CAAM should be able to use kernel's
RNG by default.
-Sumit
> Most people don't change defaults.
>
> Essentially your same argument was used for Dual_EC_DRBG; people argued it was
> okay to standardize because people had the option to choose their own constants
> if they felt the default constants were backdoored. That didn't really matter,
> though, since in practice everyone just used the default constants.
>
> >
> > > - trusted and encrypted keys aren't restricted to specific uses in the kernel
> > > (like the fscrypt-provisioning key type is) but rather are general-purpose.
> > > Hence, it may be possible to leak their contents to userspace by requesting
> > > their use for certain algorithms/features, e.g. to encrypt a dm-crypt target
> > > using a weak cipher that is vulnerable to key recovery attacks.
> >
> > The footgun is already there by allowing users to specify their own
> >
> > raw key. Users can already use $keyid for dm-crypt and then do
> >
> > $ keyctl pipe $keyid | fscryptctl add_key /mnt
> >
> > The responsibility to not reuse key material already lies with the users,
> > regardless if they handle the raw key material directly or indirectly via
> > a trusted key description/ID.
>
> Elsewhere you are claiming that "trusted" keys can never be disclosed to
> userspace. So you can't rely on userspace cooperating, right?
>
> - Eric
^ permalink raw reply
* [syzbot] kernel BUG in assoc_array_insert (2)
From: syzbot @ 2021-07-29 5:31 UTC (permalink / raw)
To: dhowells, jarkko, jmorris, keyrings, linux-kernel,
linux-security-module, serge, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 7d549995d4e0 Merge tag 'for-linus' of git://git.kernel.org..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=17577e1a300000
kernel config: https://syzkaller.appspot.com/x/.config?x=1dee114394f7d2c2
dashboard link: https://syzkaller.appspot.com/bug?extid=219c8d031f42380c907a
compiler: gcc (Debian 10.2.1-6) 10.2.1 20210110, GNU ld (GNU Binutils for Debian) 2.35.1
Unfortunately, I don't have any reproducer for this issue yet.
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+219c8d031f42380c907a@syzkaller.appspotmail.com
------------[ cut here ]------------
kernel BUG at lib/assoc_array.c:640!
invalid opcode: 0000 [#1] PREEMPT SMP KASAN
CPU: 1 PID: 9793 Comm: kworker/1:8 Not tainted 5.14.0-rc3-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: afs afs_manage_cell_work
RIP: 0010:assoc_array_insert_into_terminal_node lib/assoc_array.c:640 [inline]
RIP: 0010:assoc_array_insert+0x1e3e/0x2e70 lib/assoc_array.c:1001
Code: 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 84 c4 fe ff ff e8 3c ac e0 fd e9 ba fe ff ff e8 02 f2 9a fd <0f> 0b e8 fb f1 9a fd 0f 0b e8 f4 f1 9a fd 0f 0b e8 ed f1 9a fd 0f
RSP: 0018:ffffc90009c177b0 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 00000000000000ff RCX: 0000000000000000
RDX: ffff888087970100 RSI: ffffffff83d9cece RDI: 0000000000000003
RBP: 0000000000000011 R08: 0000000000000010 R09: 000000000000000f
R10: ffffffff83d9c0d9 R11: 000000000000000c R12: 00000000000000ff
R13: dffffc0000000000 R14: 0000000000000010 R15: 000000000000000c
FS: 0000000000000000(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f1de4db0000 CR3: 0000000020468000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
__key_link_begin+0xec/0x250 security/keys/keyring.c:1314
construct_alloc_key security/keys/request_key.c:404 [inline]
construct_key_and_link security/keys/request_key.c:499 [inline]
request_key_and_link+0x798/0x1260 security/keys/request_key.c:637
request_key_tag+0x4e/0xb0 security/keys/request_key.c:701
dns_query+0x257/0x6d0 net/dns_resolver/dns_query.c:128
afs_dns_query+0x122/0x390 fs/afs/addr_list.c:249
afs_update_cell fs/afs/cell.c:402 [inline]
afs_manage_cell fs/afs/cell.c:784 [inline]
afs_manage_cell_work+0xa05/0x11f0 fs/afs/cell.c:840
process_one_work+0x98d/0x1630 kernel/workqueue.c:2276
worker_thread+0x658/0x11f0 kernel/workqueue.c:2422
kthread+0x3e5/0x4d0 kernel/kthread.c:319
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:295
Modules linked in:
---[ end trace 02156db422e890c4 ]---
RIP: 0010:assoc_array_insert_into_terminal_node lib/assoc_array.c:640 [inline]
RIP: 0010:assoc_array_insert+0x1e3e/0x2e70 lib/assoc_array.c:1001
Code: 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 84 c4 fe ff ff e8 3c ac e0 fd e9 ba fe ff ff e8 02 f2 9a fd <0f> 0b e8 fb f1 9a fd 0f 0b e8 f4 f1 9a fd 0f 0b e8 ed f1 9a fd 0f
RSP: 0018:ffffc90009c177b0 EFLAGS: 00010293
RAX: 0000000000000000 RBX: 00000000000000ff RCX: 0000000000000000
RDX: ffff888087970100 RSI: ffffffff83d9cece RDI: 0000000000000003
RBP: 0000000000000011 R08: 0000000000000010 R09: 000000000000000f
R10: ffffffff83d9c0d9 R11: 000000000000000c R12: 00000000000000ff
R13: dffffc0000000000 R14: 0000000000000010 R15: 000000000000000c
FS: 0000000000000000(0000) GS:ffff8880b9d00000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000000308a888 CR3: 000000007ed4e000 CR4: 00000000001506e0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
^ permalink raw reply
* Re: [PATCH v2] KEYS: trusted: fix use as module when CONFIG_TCG_TPM=m
From: Ahmad Fatoum @ 2021-07-28 22:29 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar,
Sumit Garg, David Howells, Herbert Xu, David S. Miller, kernel,
Andreas Rammhold, David Gstir, Richard Weinberger, keyrings,
linux-crypto, linux-kernel, linux-security-module,
linux-integrity
In-Reply-To: <20210728215200.nfvnm5s2b27ang7i@kernel.org>
On 28.07.21 23:52, Jarkko Sakkinen wrote:
> On Tue, Jul 27, 2021 at 06:24:49AM +0200, Ahmad Fatoum wrote:
>> On 27.07.21 05:04, Jarkko Sakkinen wrote:
>>>> Reported-by: Andreas Rammhold <andreas@rammhold.de>
>>>> Fixes: 5d0682be3189 ("KEYS: trusted: Add generic trusted keys framework")
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>
>>> Is it absolutely need to do all this *just* to fix the bug?
>>>
>>> For a pure bug fix the most essential thing is to be able the backport
>>> it to stable kernels.
>>
>> Not much happened in-between, so a backport should be trivial.
>> I can provide these if needed.
>
> "not much" is not good enough. It should be "not anything".
"Not much" [code that could conflict was added in-between].
I just checked and it applies cleanly on v5.13. On the off chance
that this patch conflicts with another stable backport by the time
it's backported, I'll get a friendly automated email and send out
a rebased patch.
Cheers,
Ahmad
>
> /Jarkko
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox