Skip to contentSkip to conversation

Add capabilities file to sysfs

RFC PATCH Superseded Francis Laniel 4 participants 10 messages Jan 20, 2022 – Jan 22, 2022

Subsystems (2)

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v3 cover Add capabilities file to sysfs

Hi.

First, I hope you are fine and the same for your relatives.

Capabilities are used to check if a thread has the right to perform a given action [1].
For example, a thread with CAP_BPF set can use the bpf() syscall.

Capabilities are used in the container world.
In terms of code, several projects related to container maintain code where the capabilities are written alike include/uapi/linux/capability.h [2][3][4][5]. For these projects, their codebase should be updated when a new capability is added to the kernel.
Some other projects rely on <sys/capability.h> [6].
In this case, this header file should reflect the capabilities offered by the kernel.

So, in this series, I added a new file to sysfs:
/sys/kernel/security/capabilities.
The goal of this file is to be used by "container world" software to know kernel capabilities at run time instead of compile time.

The "file" is read-only and its content is the capability number associated with the capability name:

root@vm-amd64:~# cat /sys/kernel/security/capabilities
0       CAP_CHOWN
1       CAP_DAC_OVERRIDE
...
40      CAP_CHECKPOINT_RESTORE

The kernel already exposes the last capability number under:
/proc/sys/kernel/cap_last_cap
So, I think there should not be any issue exposing all the capabilities it offers.
If there is any, please share it as I do not want to introduce issue with this series.

Also, if you see any way to improve this series please share it as it would increase this contribution quality.

Change since v2:
* Use a char * for cap_string instead of an array, each line of this char * contains the capability number and its name.
* Move the file under /sys/kernel/security instead of /sys/kernel.

Francis Laniel (2):
capability: Add cap_string.
security/inode.c: Add capabilities file.

 include/uapi/linux/capability.h |  1 +
 kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
 security/inode.c                | 16 ++++++++++++
 3 files changed, 62 insertions(+)

Best regards and thank you in advance for your reviews.
---
[1] man capabilities
[2] https://github.com/containerd/containerd/blob/1a078e6893d07fec10a4940a5664fab21d6f7d1e/pkg/cap/cap_linux.go#L135
[3] https://github.com/moby/moby/commit/485cf38d48e7111b3d1f584d5e9eab46a902aabc#diff-2e04625b209932e74c617de96682ed72fbd1bb0d0cb9fb7c709cf47a86b6f9c1
moby relies on containerd code.
[4] https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b7f94e3a0ffb/capability/enum.go#L47
[5] https://github.com/opencontainers/runc/blob/00f56786bb220b55b41748231880ba0e6380519a/libcontainer/capabilities/capabilities.go#L12
runc relies on syndtr package.
[6] https://github.com/containers/crun/blob/fafb556f09e6ffd4690c452ff51856b880c089f1/src/libcrun/linux.c#L35

Signature · 1 line
2.30.2
#1 lore raw

RFC PATCH v3 1/2 capability: Add cap_string.

This string contains on each line the number of the capability associated to its name.
For example, first line is:
__stringify(CAP_CHOWN) "\tCAP_CHOWN\n"
which the preprocessor will replace by:
"0\tCAP_CHOWN\n"

---
 include/uapi/linux/capability.h |  1 +
 kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+)
include/uapi/linux/capability.h +1 -0
index 463d1ba2232a..115f4fef00da 100644
@@ -428,5 +428,6 @@ struct vfs_ns_cap_data {
 #define CAP_TO_INDEX(x)     ((x) >> 5)        /* 1 << 5 == bits in __u32 */
 #define CAP_TO_MASK(x)      (1 << ((x) & 31)) /* mask for indexed __u32 */
 
+extern const char *cap_string;
 
 #endif /* _UAPI_LINUX_CAPABILITY_H */
kernel/capability.c +45 -0

57 lines not on this page · raw message

Signature · 1 line
2.30.2
#2 lore raw

RFC PATCH v3 2/2 security/inode.c: Add capabilities file.

In reply to Francis Laniel

This new read-only file prints the capabilities values with their names:
cat /sys/kernel/security/capabilities
0 CAP_CHOWN
1 CAP_DAC_OVERRIDE
...
40 CAP_CHECKPOINT_RESTORE

---
 security/inode.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
security/inode.c +16 -0
index 6c326939750d..cef78b497bab 100644
@@ -21,6 +21,7 @@
 #include <linux/security.h>
 #include <linux/lsm_hooks.h>
 #include <linux/magic.h>
+#include <linux/capability.h>
 
 static struct vfsmount *mount;
 static int mount_count;
@@ -328,6 +329,19 @@ static const struct file_operations lsm_ops = {
 };
 #endif
 
+static struct dentry *capabilities_dentry;
+static ssize_t capabilities_read(struct file *unused, char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	return simple_read_from_buffer(buf, count, ppos, cap_string,
+				       strlen(cap_string));
+}
+
+static const struct file_operations capabilities_ops = {
+	.read = capabilities_read,
+	.llseek = generic_file_llseek,
+};
+
 static int __init securityfs_init(void)
 {
 	int retval;
@@ -345,6 +359,8 @@ static int __init securityfs_init(void)
 	lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
 						&lsm_ops);
 #endif
+	capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,
+					 capabilities_ops);
 	return 0;
 }
 core_initcall(securityfs_init);
Signature · 1 line
2.30.2
Acked-by on the v3 series
#6 lore raw
In reply to Francis Laniel
Quoted 46 lines · Francis Laniel

Hi.

First, I hope you are fine and the same for your relatives.

Capabilities are used to check if a thread has the right to perform a given action [1].
For example, a thread with CAP_BPF set can use the bpf() syscall.

Capabilities are used in the container world.
In terms of code, several projects related to container maintain code where the capabilities are written alike include/uapi/linux/capability.h [2][3][4][5]. For these projects, their codebase should be updated when a new capability is added to the kernel.
Some other projects rely on <sys/capability.h> [6].
In this case, this header file should reflect the capabilities offered by the kernel.

So, in this series, I added a new file to sysfs:
/sys/kernel/security/capabilities.
The goal of this file is to be used by "container world" software to know kernel capabilities at run time instead of compile time.

The "file" is read-only and its content is the capability number associated with the capability name:

root@vm-amd64:~# cat /sys/kernel/security/capabilities
0       CAP_CHOWN
1       CAP_DAC_OVERRIDE
...
40      CAP_CHECKPOINT_RESTORE

The kernel already exposes the last capability number under:
/proc/sys/kernel/cap_last_cap
So, I think there should not be any issue exposing all the capabilities it offers.
If there is any, please share it as I do not want to introduce issue with this series.

Also, if you see any way to improve this series please share it as it would increase this contribution quality.

Change since v2:
* Use a char * for cap_string instead of an array, each line of this char * contains the capability number and its name.
* Move the file under /sys/kernel/security instead of /sys/kernel.

Quoted 8 lines

Francis Laniel (2):
capability: Add cap_string.
security/inode.c: Add capabilities file.

  include/uapi/linux/capability.h |  1 +
  kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
  security/inode.c                | 16 ++++++++++++
  3 files changed, 62 insertions(+)

For the series:

Quoted 12 lines

Best regards and thank you in advance for your reviews.
---
[1] man capabilities
[2] https://github.com/containerd/containerd/blob/1a078e6893d07fec10a4940a5664fab21d6f7d1e/pkg/cap/cap_linux.go#L135
[3] https://github.com/moby/moby/commit/485cf38d48e7111b3d1f584d5e9eab46a902aabc#diff-2e04625b209932e74c617de96682ed72fbd1bb0d0cb9fb7c709cf47a86b6f9c1
moby relies on containerd code.
[4] https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b7f94e3a0ffb/capability/enum.go#L47
[5] https://github.com/opencontainers/runc/blob/00f56786bb220b55b41748231880ba0e6380519a/libcontainer/capabilities/capabilities.go#L12
runc relies on syndtr package.
[6] https://github.com/containers/crun/blob/fafb556f09e6ffd4690c452ff51856b880c089f1/src/libcrun/linux.c#L35

#7 lore raw
Quoted 54 lines · Le jeudi 20 janvier 2022, 19:09:50 CET Casey Schaufler a écrit :
Quoted 53 lines · Francis Laniel

Hi.

First, I hope you are fine and the same for your relatives.

Capabilities are used to check if a thread has the right to perform a given
action [1].
For example, a thread with CAP_BPF set can use the bpf() syscall.

Capabilities are used in the container world.
In terms of code, several projects related to container maintain code where the capabilities are written alike include/uapi/linux/capability.h [2][3][4][5]. For these projects, their codebase should be updated when a new capability is added to the kernel.
Some other projects rely on <sys/capability.h> [6].
In this case, this header file should reflect the capabilities offered by the kernel.

So, in this series, I added a new file to sysfs:
/sys/kernel/security/capabilities.
The goal of this file is to be used by "container world" software to know kernel capabilities at run time instead of compile time.

The "file" is read-only and its content is the capability number associated with the capability name:

root@vm-amd64:~# cat /sys/kernel/security/capabilities
0       CAP_CHOWN
1       CAP_DAC_OVERRIDE
...
40      CAP_CHECKPOINT_RESTORE

The kernel already exposes the last capability number under:
/proc/sys/kernel/cap_last_cap
So, I think there should not be any issue exposing all the capabilities it offers.
If there is any, please share it as I do not want to introduce issue with this series.

Also, if you see any way to improve this series please share it as it would
increase this contribution quality.

Change since v2:
* Use a char * for cap_string instead of an array, each line of this char *
contains the capability number and its name.
* Move the file under /sys/kernel/security instead of /sys/kernel.

Francis Laniel (2):
capability: Add cap_string.
security/inode.c: Add capabilities file.

Quoted 8 lines
Quoted 4 lines
  include/uapi/linux/capability.h |  1 +
  kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
  security/inode.c                | 16 ++++++++++++
  3 files changed, 62 insertions(+)

For the series:

Thank you!
I will wait to get some comments on the v3 and send a v4 with your tag added!

Quoted 18 lines
Quoted 18 lines

Best regards and thank you in advance for your reviews.
---
[1] man capabilities
[2]
https://github.com/containerd/containerd/blob/1a078e6893d07fec10a4940a566
4fab21d6f7d1e/pkg/cap/cap_linux.go#L135 [3]
https://github.com/moby/moby/commit/485cf38d48e7111b3d1f584d5e9eab46a902a
abc#diff-2e04625b209932e74c617de96682ed72fbd1bb0d0cb9fb7c709cf47a86b6f9c1 moby relies on containerd code.
[4]
https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b
7f94e3a0ffb/capability/enum.go#L47 [5]
https://github.com/opencontainers/runc/blob/00f56786bb220b55b41748231880b
a0e6380519a/libcontainer/capabilities/capabilities.go#L12 runc relies on syndtr package.
[6]
https://github.com/containers/crun/blob/fafb556f09e6ffd4690c452ff51856b88
0c089f1/src/libcrun/linux.c#L35

#8 lore raw
In reply to Francis Laniel
Quoted 19 lines · Francis Laniel

Hi.

First, I hope you are fine and the same for your relatives.

Capabilities are used to check if a thread has the right to perform a given action [1].
For example, a thread with CAP_BPF set can use the bpf() syscall.

Capabilities are used in the container world.
In terms of code, several projects related to container maintain code where the capabilities are written alike include/uapi/linux/capability.h [2][3][4][5]. For these projects, their codebase should be updated when a new capability is added to the kernel.
Some other projects rely on <sys/capability.h> [6].
In this case, this header file should reflect the capabilities offered by the kernel.

Quoted 2 lines

So, in this series, I added a new file to sysfs:
/sys/kernel/security/capabilities.

Actually that is a file in securityfs. Which is related but slightly different. For sysfs this would be immediately unacceptable as it breaks the one value per file rule. I don't know what the rules are for securityfs but I do know files that contain many many lines and get very large tend to be problematic in both their kernel implementation and in userspace parsing speed.

So I am looking for what the advantage of this file that justifies the cost of maintaining it.

Quoted 2 lines

The goal of this file is to be used by "container world" software to know kernel capabilities at run time instead of compile time.

I don't understand the problem you are trying to solve. If the software needs to updated what benefit is there for all of the information to be available at runtime?

Quoted 10 lines

The "file" is read-only and its content is the capability number associated with the capability name:

root@vm-amd64:~# cat /sys/kernel/security/capabilities
0       CAP_CHOWN
1       CAP_DAC_OVERRIDE
...
40      CAP_CHECKPOINT_RESTORE
Quoted 6 lines

The kernel already exposes the last capability number under:
/proc/sys/kernel/cap_last_cap
So, I think there should not be any issue exposing all the capabilities it offers.
If there is any, please share it as I do not want to introduce issue with this series.

The mapping between capabilities and numbers should never change it is ABI. I seem to remember a version number in the file capability so that if the mappings do change that number can be changed in a way that existing software is not confused.

What is the advantage in printing all of the mappings?

Quoted 19 lines

Also, if you see any way to improve this series please share it as it would increase this contribution quality.

Change since v2:
* Use a char * for cap_string instead of an array, each line of this char * contains the capability number and its name.
* Move the file under /sys/kernel/security instead of /sys/kernel.

Francis Laniel (2):
capability: Add cap_string.
security/inode.c: Add capabilities file.

 include/uapi/linux/capability.h |  1 +
 kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
 security/inode.c                | 16 ++++++++++++
 3 files changed, 62 insertions(+)
Quoted 10 lines

Best regards and thank you in advance for your reviews.
---
[1] man capabilities
[2] https://github.com/containerd/containerd/blob/1a078e6893d07fec10a4940a5664fab21d6f7d1e/pkg/cap/cap_linux.go#L135
[3] https://github.com/moby/moby/commit/485cf38d48e7111b3d1f584d5e9eab46a902aabc#diff-2e04625b209932e74c617de96682ed72fbd1bb0d0cb9fb7c709cf47a86b6f9c1
moby relies on containerd code.
[4] https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b7f94e3a0ffb/capability/enum.go#L47
[5] https://github.com/opencontainers/runc/blob/00f56786bb220b55b41748231880ba0e6380519a/libcontainer/capabilities/capabilities.go#L12
runc relies on syndtr package.
[6] https://github.com/containers/crun/blob/fafb556f09e6ffd4690c452ff51856b880c089f1/src/libcrun/linux.c#L35

Eric

#9 lore raw

Hi.

Quoted 24 lines · Le jeudi 20 janvier 2022, 19:20:15 CET Eric W. Biederman a écrit :
Quoted 20 lines · Francis Laniel

Hi.

First, I hope you are fine and the same for your relatives.

Capabilities are used to check if a thread has the right to perform a given
action [1].
For example, a thread with CAP_BPF set can use the bpf() syscall.

Capabilities are used in the container world.
In terms of code, several projects related to container maintain code where the capabilities are written alike include/uapi/linux/capability.h [2][3][4][5]. For these projects, their codebase should be updated when a new capability is added to the kernel.
Some other projects rely on <sys/capability.h> [6].
In this case, this header file should reflect the capabilities offered by the kernel.

Quoted 2 lines

So, in this series, I added a new file to sysfs:
/sys/kernel/security/capabilities.

Quoted 6 lines

Actually that is a file in securityfs. Which is related but slightly different. For sysfs this would be immediately unacceptable as it breaks the one value per file rule. I don't know what the rules are for securityfs but I do know files that contain many many lines and get very large tend to be problematic in both their kernel implementation and in userspace parsing speed.

I was not aware of the sysfs rule, thank you for sharing it to me, I will add it to my "kernel knowledge" and will make use of it in the future!

Quoted 9 lines

So I am looking for what the advantage of this file that justifies the cost of maintaining it.

Quoted 2 lines

The goal of this file is to be used by "container world" software to know kernel capabilities at run time instead of compile time.

I don't understand the problem you are trying to solve. If the software needs to updated what benefit is there for all of the information to be available at runtime?

Actually, software like containerd hardcodes all the capabilities the kernel knows based on a per-version approach [1].
So if a new capabilities, say CAP_NEW, is added to the kernel, containerd code would become something like this:

	// caps59 is the caps of kernel 5.9 (41 entries)
	caps59     = append(caps58, "CAP_CHECKPOINT_RESTORE")
	// caps59 is the caps of kernel 5.17 (42 entries)
	cap517    = append(caps59, "CAP_NEW")
	capsLatest = caps517

This approach has two drawbacks:
1. A user which wants to use CAP_NEW would need to use kernel 5.17 but also a version of containerd which knows about CAP_NEW. So, this user would need to wait for containerd code to be updated (or to modify it by him/herself and compiles it).
2. Containerd maintainers would need to change their code to add CAP_NEW.

It would be easier for everyone if the kernel exposes its capabilities, thus containerd code would become something like this:

	caps, err := readCapsFromFile("/sys/kernel/security/capabilities")
	if err {
		caps = capsLatest
	}

This approach addresses the first drawback I quoted above and partly the second one:
1. If user kernel was compiled with CONFIG_SECURITYFS, he/she does not need to wait the last version of containerd to use CAP_NEW, as containerd would read the kernel capabilities from "/sys/kernel/security/capabilities".
2. Containerd maintainers would not need to quickly update their code to reflect last kernel change.
I agree they would still need to update their code in case /sys/kernel/ security/capabilities does not exist.

To conclude, this series adds a bit of code in the kernel to make userland life easier while not doing nasty things inside the kernel. What do you think about it?

Quoted 16 lines
Quoted 9 lines

The "file" is read-only and its content is the capability number associated with the capability name:

root@vm-amd64:~# cat /sys/kernel/security/capabilities
0       CAP_CHOWN
1       CAP_DAC_OVERRIDE
...
40      CAP_CHECKPOINT_RESTORE
Quoted 6 lines

The kernel already exposes the last capability number under:
/proc/sys/kernel/cap_last_cap
So, I think there should not be any issue exposing all the capabilities it offers.
If there is any, please share it as I do not want to introduce issue with this series.

Quoted 6 lines

The mapping between capabilities and numbers should never change it is ABI. I seem to remember a version number in the file capability so that if the mappings do change that number can be changed in a way that existing software is not confused.

What is the advantage in printing all of the mappings?

The printing of all the mappings can be used by containerd code to know about the capabilities the kernel knows.
For example, the above mentioned function readCapsFromFile() could be implemented like this:

func readCapsFromFile(path string) (string[], err) {
	var capabilities string[];
	// Open the file.
	re := regexp.MustCompile(`(\d+)\t(CAP_\w+)`)
    	for line /*...*/ {
		matches := re.FindStringSubmatch(line)
		// To simplify, I will make the hypothesis the regex matched and 
		// everything is OK.
		// matches[0] contains the whole line expect final '\n'.
		id := strconv.Atoi(matches[1])
		name := matches[2]
		capabilities[id] = name
   	 }
	// Close the file.
	return capabilities, nil
}

It will mainly parse the file output to create the capabilities array.

Quoted 38 lines
Quoted 30 lines

Also, if you see any way to improve this series please share it as it would
increase this contribution quality.

Change since v2:
* Use a char * for cap_string instead of an array, each line of this char *
contains the capability number and its name.
* Move the file under /sys/kernel/security instead of /sys/kernel.

Francis Laniel (2):
capability: Add cap_string.
security/inode.c: Add capabilities file.

 include/uapi/linux/capability.h |  1 +
 kernel/capability.c             | 45 +++++++++++++++++++++++++++++++++
 security/inode.c                | 16 ++++++++++++
 3 files changed, 62 insertions(+)

Best regards and thank you in advance for your reviews.
---
[1] man capabilities
[2]
https://github.com/containerd/containerd/blob/1a078e6893d07fec10a4940a566
4fab21d6f7d1e/pkg/cap/cap_linux.go#L135 [3]
https://github.com/moby/moby/commit/485cf38d48e7111b3d1f584d5e9eab46a902a
abc#diff-2e04625b209932e74c617de96682ed72fbd1bb0d0cb9fb7c709cf47a86b6f9c1 moby relies on containerd code.
[4]

Quoted 8 lines

https://github.com/syndtr/gocapability/blob/42c35b4376354fd554efc7ad35e0b
7f94e3a0ffb/capability/enum.go#L47 [5]
https://github.com/opencontainers/runc/blob/00f56786bb220b55b41748231880b
a0e6380519a/libcontainer/capabilities/capabilities.go#L12 runc relies on syndtr package.
[6]
https://github.com/containers/crun/blob/fafb556f09e6ffd4690c452ff51856b88
0c089f1/src/libcrun/linux.c#L35

Quoted 1 line

Eric

Best regards.
---
[1] https://github.com/containerd/containerd/blob/
1a078e6893d07fec10a4940a5664fab21d6f7d1e/pkg/cap/cap_linux.go#L181

#3 lore raw

Re: [RFC PATCH v3 2/2] security/inode.c: Add capabilities file.

In reply to Francis Laniel
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]

Hi Francis,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linux/master]
[also build test ERROR on linus/master jmorris-security/next-testing v5.16 next-20220120] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Francis-Laniel/Add-capabilities-file-to-sysfs/20220121-020524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1d1df41c5a33359a00e919d54eaebfb789711fdc
config: x86_64-randconfig-a011 (https://download.01.org/0day-ci/archive/20220121/202201211107.0Qa4Y0bI-lkp(a)intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
        # https://github.com/0day-ci/linux/commit/48b37f6e79a73709d7bcf17ea19464073f5da9b6
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Francis-Laniel/Add-capabilities-file-to-sysfs/20220121-020524
        git checkout 48b37f6e79a73709d7bcf17ea19464073f5da9b6
        # save the config file to linux build tree
        mkdir build_dir
        make W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

All error/warnings (new ones prefixed by >>):

security/inode.c: In function 'securityfs_init':

>> security/inode.c:362:24: error: implicit declaration of function 'securityfs'; did you mean 'security_bpf'? [-Werror=implicit-function-declaration]
     362 |  capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,
         |                        ^~~~~~~~~~
         |                        security_bpf
>> security/inode.c:362:22: warning: assignment to 'struct dentry *' from 'int' makes pointer from integer without a cast [-Wint-conversion]

362 | capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,
| ^
cc1: some warnings being treated as errors

vim +362 security/inode.c

344
345 static int __init securityfs_init(void)
346 {
347 int retval;
348
349 retval = sysfs_create_mount_point(kernel_kobj, "security");
350 if (retval)
351 return retval;
352
353 retval = register_filesystem(&fs_type);
354 if (retval) {
355 sysfs_remove_mount_point(kernel_kobj, "security");
356 return retval;
357 }
358 #ifdef CONFIG_SECURITY
359 lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
360 &lsm_ops);
361 #endif

 > 362		capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

#4 lore raw

Re: [RFC PATCH v3 2/2] security/inode.c: Add capabilities file.

In reply to Francis Laniel

Hi.

Quoted 45 lines · Le jeudi 20 janvier 2022, 19:01:16 CET Francis Laniel a écrit :

This new read-only file prints the capabilities values with their names:
cat /sys/kernel/security/capabilities
0 CAP_CHOWN
1 CAP_DAC_OVERRIDE
...
40 CAP_CHECKPOINT_RESTORE

---
 security/inode.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
security/inode.c +14 -0
index 6c326939750d..cef78b497bab 100644
@@ -21,6 +21,7 @@
 #include <linux/security.h>
 #include <linux/lsm_hooks.h>
 #include <linux/magic.h>
+#include <linux/capability.h>
 
 static struct vfsmount *mount;
 static int mount_count;
@@ -328,6 +329,19 @@ static const struct file_operations lsm_ops = {
 };
 #endif
 
+static struct dentry *capabilities_dentry;
+static ssize_t capabilities_read(struct file *unused, char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	return simple_read_from_buffer(buf, count, ppos, cap_string,
+				       strlen(cap_string));
+}
+
+static const struct file_operations capabilities_ops = {
+	.read = capabilities_read,
+	.llseek = generic_file_llseek,
+};
+
 static int __init securityfs_init(void)
 {
 	int retval;

security/inode.c static int __init securityfs_init(void) · line 363

Quoted 5 lines
security/inode.c +2 -0
@@ -345,6 +359,8 @@ static int __init securityfs_init(void)
 	lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
 						&lsm_ops);
 #endif
+	capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,
+					 capabilities_ops);

Sorry, I sent the old version of the patch and did not fixup this... Kernel robot kindly show me this error.
I swear the output in the cover letter was done on the compiled kernel within a VM.

I will send a v4 correcting this but I will wait to get some reviews on v3 to not send to not generate too much traffic here.

security/inode.c static int __init securityfs_init(void) · line 366

Quoted 3 lines
security/inode.c +0 -0
 	return 0;
 }
 core_initcall(securityfs_init);

Best regards.

#5 lore raw

Re: [RFC PATCH v3 2/2] security/inode.c: Add capabilities file.

In reply to Francis Laniel
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]

Hi Francis,

[FYI, it's a private test report for your RFC patch.]
[auto build test ERROR on linux/master]
[also build test ERROR on linus/master jmorris-security/next-testing v5.16 next-20220121] [If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Francis-Laniel/Add-capabilities-file-to-sysfs/20220121-020524
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1d1df41c5a33359a00e919d54eaebfb789711fdc
config: riscv-randconfig-r026-20220120 (https://download.01.org/0day-ci/archive/20220122/202201222235.SOQYCPHW-lkp(a)intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project f7b7138a62648f4019c55e4671682af1f851f295)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/48b37f6e79a73709d7bcf17ea19464073f5da9b6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Francis-Laniel/Add-capabilities-file-to-sysfs/20220121-020524 git checkout 48b37f6e79a73709d7bcf17ea19464073f5da9b6
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate

All error/warnings (new ones prefixed by >>):

In file included from security/inode.c:18:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/riscv/include/asm/io.h:136:
include/asm-generic/io.h:464:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]

           val = __raw_readb(PCI_IOBASE + addr);
                             ~~~~~~~~~~ ^
   include/asm-generic/io.h:477:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
   #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
                                                     ^
   In file included from security/inode.c:18:
   In file included from include/linux/pagemap.h:11:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
                                                           ~~~~~~~~~~ ^
   include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
   #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
                                                     ^
   In file included from security/inode.c:18:
   In file included from include/linux/pagemap.h:11:
   In file included from include/linux/highmem.h:11:
   In file included from include/linux/hardirq.h:11:
   In file included from ./arch/riscv/include/generated/asm/hardirq.h:1:
   In file included from include/asm-generic/hardirq.h:17:
   In file included from include/linux/irq.h:20:
   In file included from include/linux/io.h:13:
   In file included from arch/riscv/include/asm/io.h:136:
   include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writeb(value, PCI_IOBASE + addr);
                               ~~~~~~~~~~ ^
   include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
                                                         ~~~~~~~~~~ ^
   include/asm-generic/io.h:1024:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
           return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
                                                     ~~~~~~~~~~ ^
>> security/inode.c:362:24: error: implicit declaration of function 'securityfs' [-Werror,-Wimplicit-function-declaration]
           capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,
                                 ^
>> security/inode.c:362:22: warning: incompatible integer to pointer conversion assigning to 'struct dentry *' from 'int' [-Wint-conversion]

capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
8 warnings and 1 error generated.

vim +/securityfs +362 security/inode.c

344
345 static int __init securityfs_init(void)
346 {
347 int retval;
348
349 retval = sysfs_create_mount_point(kernel_kobj, "security");
350 if (retval)
351 return retval;
352
353 retval = register_filesystem(&fs_type);
354 if (retval) {
355 sysfs_remove_mount_point(kernel_kobj, "security");
356 return retval;
357 }
358 #ifdef CONFIG_SECURITY
359 lsm_dentry = securityfs_create_file("lsm", 0444, NULL, NULL,
360 &lsm_ops);
361 #endif

 > 362		capabilities_dentry = securityfs("capabilities", 0444, NULL, NULL,

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org