| Message ID | 20221001154908.49665-5-gnoack3000@gmail.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | landlock: truncate support | expand |
Hi Günther, On Sat, Oct 01, 2022 at 05:49:03PM +0200, Günther Noack wrote: > Introduce the LANDLOCK_ACCESS_FS_TRUNCATE flag for file truncation. > > This flag hooks into the path_truncate LSM hook and covers file > truncation using truncate(2), ftruncate(2), open(2) with O_TRUNC, as > well as creat(). > > This change also increments the Landlock ABI version, updates > corresponding selftests, and updates code documentation to document > the flag. > > The following operations are restricted: > > open(): requires the LANDLOCK_ACCESS_FS_TRUNCATE right if a file gets > implicitly truncated as part of the open() (e.g. using O_TRUNC). > > Notable special cases: > * open(..., O_RDONLY|O_TRUNC) can truncate files as well in Linux > * open() with O_TRUNC does *not* need the TRUNCATE right when it > creates a new file. > > truncate() (on a path): requires the LANDLOCK_ACCESS_FS_TRUNCATE > right. > > ftruncate() (on a file): requires that the file had the TRUNCATE right > when it was previously opened. > > Signed-off-by: Günther Noack <gnoack3000@gmail.com> I just bisected a crash in QEMU with Debian's arm64 configuration to this change in -next as commit b40deebe7679 ("landlock: Support file truncation"), which I was able to reproduce like so: $ mkdir -p build/deb $ cd build/deb $ curl -LSsO http://ftp.us.debian.org/debian/pool/main/l/linux-signed-arm64/linux-image-5.19.0-2-arm64_5.19.11-1_arm64.deb $ ar x linux-image-5.19.0-2-arm64_5.19.11-1_arm64.deb $ tar xJf data.tar.xz $ cp boot/config-5.19.0-2-arm64 ../.config $ cd ../.. $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build olddefconfig Image.gz $ qemu-system-aarch64 \ -machine virt,gic-version=max,virtualization=true \ -cpu max,pauth-impdef=true \ -kernel build/arch/arm64/boot/Image.gz \ -append "console=ttyAMA0 earlycon" \ -display none \ -initrd .../rootfs.cpio \ -m 512m \ -nodefaults \ -no-reboot \ -serial mon:stdio ... [ 0.000000] Linux version 6.0.0-rc7+ (nathan@dev-arch.thelio-3990X) (aarch64-linux-gnu-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 2.39) #1 SMP Tue Oct 4 12:48:50 MST 2022 ... [ 0.518570] Unable to handle kernel paging request at virtual address ffff00000851ff8a [ 0.518785] Mem abort info: [ 0.518867] ESR = 0x0000000097c0c061 [ 0.519001] EC = 0x25: DABT (current EL), IL = 32 bits [ 0.519155] SET = 0, FnV = 0 [ 0.519267] EA = 0, S1PTW = 0 [ 0.519386] FSC = 0x21: alignment fault [ 0.519524] Data abort info: [ 0.519615] Access size = 8 byte(s) [ 0.519722] SSE = 0, SRT = 0 [ 0.519817] SF = 1, AR = 1 [ 0.519920] CM = 0, WnR = 1 [ 0.520040] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000041711000 [ 0.520225] [ffff00000851ff8a] pgd=180000005fff8003, p4d=180000005fff8003, pud=180000005fff7003, pmd=180000005ffbd003, pte=006800004851ff07 [ 0.521121] Internal error: Oops: 97c0c061 [#1] SMP [ 0.521364] Modules linked in: [ 0.521592] CPU: 0 PID: 9 Comm: kworker/u2:0 Not tainted 6.0.0-rc7+ #1 [ 0.521863] Hardware name: linux,dummy-virt (DT) [ 0.522325] Workqueue: events_unbound async_run_entry_fn [ 0.522973] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 0.523193] pc : apparmor_file_alloc_security+0x98/0x1e0 [ 0.523431] lr : apparmor_file_alloc_security+0x48/0x1e0 [ 0.523594] sp : ffff800008093960 [ 0.523708] x29: ffff800008093960 x28: ffff800008093b30 x27: ffff000002602600 [ 0.523978] x26: ffffd79796ecf8c0 x25: ffff00000241e705 x24: ffffd79797d98068 [ 0.524199] x23: ffff00000851ff82 x22: ffff00000851ff80 x21: 0000000000000002 [ 0.524431] x20: ffffd79796ff5000 x19: ffff00000241ceb0 x18: ffffffffffffffff [ 0.524647] x17: 000000000000003f x16: ffffd79797678008 x15: 0000000000000000 [ 0.524850] x14: 0000000000000001 x13: 0000000000000000 x12: 0000000000000006 [ 0.525087] x11: ffff00001feef940 x10: ffffd7979768f8a0 x9 : ffffd79796c1e51c [ 0.525325] x8 : ffff00000851ffa0 x7 : 0000000000000000 x6 : 0000000000001e0b [ 0.525531] x5 : ffff00000851ff80 x4 : ffff800008093990 x3 : ffff000002419700 [ 0.525745] x2 : 0000000000000001 x1 : ffff00000851ff8a x0 : ffff00000241ceb0 [ 0.526034] Call trace: [ 0.526166] apparmor_file_alloc_security+0x98/0x1e0 [ 0.526424] security_file_alloc+0x6c/0xf0 [ 0.526570] __alloc_file+0x5c/0xf0 [ 0.526699] alloc_empty_file+0x68/0x10c [ 0.526816] path_openat+0x50/0x106c [ 0.526929] do_filp_open+0x88/0x13c [ 0.527041] filp_open+0x110/0x1b0 [ 0.527143] do_name+0xbc/0x230 [ 0.527256] write_buffer+0x40/0x60 [ 0.527359] unpack_to_rootfs+0x100/0x2bc [ 0.527479] do_populate_rootfs+0x70/0x134 [ 0.527602] async_run_entry_fn+0x40/0x1c0 [ 0.527723] process_one_work+0x1f4/0x450 [ 0.527851] worker_thread+0x188/0x4c0 [ 0.527980] kthread+0xe0/0xe4 [ 0.528066] ret_from_fork+0x10/0x20 [ 0.528317] Code: 52800002 d2800000 d2800013 910022e1 (c89ffc20) [ 0.528736] ---[ end trace 0000000000000000 ]--- ... A rootfs is available at [1] but I don't think it should be necessary for reproducing this. If there is any additional information I can provide or patches I can test, I am more than happy to do so! [1]: https://github.com/ClangBuiltLinux/boot-utils/raw/bf2fd3500d87f78a914bfc3769b2240f5632e5b9/images/arm64/rootfs.cpio.zst Cheers, Nathan
On 04/10/2022 21:56, Nathan Chancellor wrote: > Hi Günther, > > On Sat, Oct 01, 2022 at 05:49:03PM +0200, Günther Noack wrote: >> Introduce the LANDLOCK_ACCESS_FS_TRUNCATE flag for file truncation. >> >> This flag hooks into the path_truncate LSM hook and covers file >> truncation using truncate(2), ftruncate(2), open(2) with O_TRUNC, as >> well as creat(). >> >> This change also increments the Landlock ABI version, updates >> corresponding selftests, and updates code documentation to document >> the flag. >> >> The following operations are restricted: >> >> open(): requires the LANDLOCK_ACCESS_FS_TRUNCATE right if a file gets >> implicitly truncated as part of the open() (e.g. using O_TRUNC). >> >> Notable special cases: >> * open(..., O_RDONLY|O_TRUNC) can truncate files as well in Linux >> * open() with O_TRUNC does *not* need the TRUNCATE right when it >> creates a new file. >> >> truncate() (on a path): requires the LANDLOCK_ACCESS_FS_TRUNCATE >> right. >> >> ftruncate() (on a file): requires that the file had the TRUNCATE right >> when it was previously opened. >> >> Signed-off-by: Günther Noack <gnoack3000@gmail.com> > > I just bisected a crash in QEMU with Debian's arm64 configuration to > this change in -next as commit b40deebe7679 ("landlock: Support file > truncation"), which I was able to reproduce like so: Thanks for the report Nathan. I've found an issue in this patch and fixed it in -next with this (rebased) commit: https://git.kernel.org/mic/c/b40deebe7679b05d4852488ef531e189a9621f2e You should already have this update since I pushed it Monday. Please let us know if this fixed the issue. > > $ mkdir -p build/deb > > $ cd build/deb > > $ curl -LSsO http://ftp.us.debian.org/debian/pool/main/l/linux-signed-arm64/linux-image-5.19.0-2-arm64_5.19.11-1_arm64.deb > > $ ar x linux-image-5.19.0-2-arm64_5.19.11-1_arm64.deb > > $ tar xJf data.tar.xz > > $ cp boot/config-5.19.0-2-arm64 ../.config > > $ cd ../.. > > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build olddefconfig Image.gz > > $ qemu-system-aarch64 \ > -machine virt,gic-version=max,virtualization=true \ > -cpu max,pauth-impdef=true \ > -kernel build/arch/arm64/boot/Image.gz \ > -append "console=ttyAMA0 earlycon" \ > -display none \ > -initrd .../rootfs.cpio \ > -m 512m \ > -nodefaults \ > -no-reboot \ > -serial mon:stdio > ... > [ 0.000000] Linux version 6.0.0-rc7+ (nathan@dev-arch.thelio-3990X) (aarch64-linux-gnu-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 2.39) #1 SMP Tue Oct 4 12:48:50 MST 2022 > ... > [ 0.518570] Unable to handle kernel paging request at virtual address ffff00000851ff8a > [ 0.518785] Mem abort info: > [ 0.518867] ESR = 0x0000000097c0c061 > [ 0.519001] EC = 0x25: DABT (current EL), IL = 32 bits > [ 0.519155] SET = 0, FnV = 0 > [ 0.519267] EA = 0, S1PTW = 0 > [ 0.519386] FSC = 0x21: alignment fault > [ 0.519524] Data abort info: > [ 0.519615] Access size = 8 byte(s) > [ 0.519722] SSE = 0, SRT = 0 > [ 0.519817] SF = 1, AR = 1 > [ 0.519920] CM = 0, WnR = 1 > [ 0.520040] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000041711000 > [ 0.520225] [ffff00000851ff8a] pgd=180000005fff8003, p4d=180000005fff8003, pud=180000005fff7003, pmd=180000005ffbd003, pte=006800004851ff07 > [ 0.521121] Internal error: Oops: 97c0c061 [#1] SMP > [ 0.521364] Modules linked in: > [ 0.521592] CPU: 0 PID: 9 Comm: kworker/u2:0 Not tainted 6.0.0-rc7+ #1 > [ 0.521863] Hardware name: linux,dummy-virt (DT) > [ 0.522325] Workqueue: events_unbound async_run_entry_fn > [ 0.522973] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > [ 0.523193] pc : apparmor_file_alloc_security+0x98/0x1e0 > [ 0.523431] lr : apparmor_file_alloc_security+0x48/0x1e0 > [ 0.523594] sp : ffff800008093960 > [ 0.523708] x29: ffff800008093960 x28: ffff800008093b30 x27: ffff000002602600 > [ 0.523978] x26: ffffd79796ecf8c0 x25: ffff00000241e705 x24: ffffd79797d98068 > [ 0.524199] x23: ffff00000851ff82 x22: ffff00000851ff80 x21: 0000000000000002 > [ 0.524431] x20: ffffd79796ff5000 x19: ffff00000241ceb0 x18: ffffffffffffffff > [ 0.524647] x17: 000000000000003f x16: ffffd79797678008 x15: 0000000000000000 > [ 0.524850] x14: 0000000000000001 x13: 0000000000000000 x12: 0000000000000006 > [ 0.525087] x11: ffff00001feef940 x10: ffffd7979768f8a0 x9 : ffffd79796c1e51c > [ 0.525325] x8 : ffff00000851ffa0 x7 : 0000000000000000 x6 : 0000000000001e0b > [ 0.525531] x5 : ffff00000851ff80 x4 : ffff800008093990 x3 : ffff000002419700 > [ 0.525745] x2 : 0000000000000001 x1 : ffff00000851ff8a x0 : ffff00000241ceb0 > [ 0.526034] Call trace: > [ 0.526166] apparmor_file_alloc_security+0x98/0x1e0 > [ 0.526424] security_file_alloc+0x6c/0xf0 > [ 0.526570] __alloc_file+0x5c/0xf0 > [ 0.526699] alloc_empty_file+0x68/0x10c > [ 0.526816] path_openat+0x50/0x106c > [ 0.526929] do_filp_open+0x88/0x13c > [ 0.527041] filp_open+0x110/0x1b0 > [ 0.527143] do_name+0xbc/0x230 > [ 0.527256] write_buffer+0x40/0x60 > [ 0.527359] unpack_to_rootfs+0x100/0x2bc > [ 0.527479] do_populate_rootfs+0x70/0x134 > [ 0.527602] async_run_entry_fn+0x40/0x1c0 > [ 0.527723] process_one_work+0x1f4/0x450 > [ 0.527851] worker_thread+0x188/0x4c0 > [ 0.527980] kthread+0xe0/0xe4 > [ 0.528066] ret_from_fork+0x10/0x20 > [ 0.528317] Code: 52800002 d2800000 d2800013 910022e1 (c89ffc20) > [ 0.528736] ---[ end trace 0000000000000000 ]--- > ... > > A rootfs is available at [1] but I don't think it should be necessary > for reproducing this. If there is any additional information I can > provide or patches I can test, I am more than happy to do so! > > [1]: https://github.com/ClangBuiltLinux/boot-utils/raw/bf2fd3500d87f78a914bfc3769b2240f5632e5b9/images/arm64/rootfs.cpio.zst > > Cheers, > Nathan
On 01/10/2022 17:49, Günther Noack wrote: > Introduce the LANDLOCK_ACCESS_FS_TRUNCATE flag for file truncation. > > This flag hooks into the path_truncate LSM hook and covers file > truncation using truncate(2), ftruncate(2), open(2) with O_TRUNC, as > well as creat(). > > This change also increments the Landlock ABI version, updates > corresponding selftests, and updates code documentation to document > the flag. > > The following operations are restricted: > > open(): requires the LANDLOCK_ACCESS_FS_TRUNCATE right if a file gets > implicitly truncated as part of the open() (e.g. using O_TRUNC). > > Notable special cases: > * open(..., O_RDONLY|O_TRUNC) can truncate files as well in Linux > * open() with O_TRUNC does *not* need the TRUNCATE right when it > creates a new file. > > truncate() (on a path): requires the LANDLOCK_ACCESS_FS_TRUNCATE > right. > > ftruncate() (on a file): requires that the file had the TRUNCATE right > when it was previously opened. > > Signed-off-by: Günther Noack <gnoack3000@gmail.com> > --- > include/uapi/linux/landlock.h | 21 +++- > security/landlock/fs.c | 102 +++++++++++++++++-- > security/landlock/fs.h | 24 +++++ > security/landlock/limits.h | 2 +- > security/landlock/setup.c | 1 + > security/landlock/syscalls.c | 2 +- > tools/testing/selftests/landlock/base_test.c | 2 +- > tools/testing/selftests/landlock/fs_test.c | 7 +- > 8 files changed, 144 insertions(+), 17 deletions(-) > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > index 23df4e0e8ace..d830cdfdbe56 100644 > --- a/include/uapi/linux/landlock.h > +++ b/include/uapi/linux/landlock.h > @@ -95,8 +95,19 @@ struct landlock_path_beneath_attr { > * A file can only receive these access rights: > * > * - %LANDLOCK_ACCESS_FS_EXECUTE: Execute a file. > - * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. > + * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. Note that > + * you might additionally need the `LANDLOCK_ACCESS_FS_TRUNCATE` right in %LANDLOCK_ACCESS_FS_TRUNCATE > + * order to overwrite files with :manpage:`open(2)` using `O_TRUNC` or > + * :manpage:`creat(2)`. > * - %LANDLOCK_ACCESS_FS_READ_FILE: Open a file with read access. > + * - %LANDLOCK_ACCESS_FS_TRUNCATE: Truncate a file with :manpage:`truncate(2)`, > + * :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with > + * `O_TRUNC`. Whether an opened file can be truncated with %O_TRUNC > + * :manpage:`ftruncate(2)` is determined during :manpage:`open(2)`, in the > + * same way as read and write permissions are checked during > + * :manpage:`open(2)` using %LANDLOCK_ACCESS_FS_READ_FILE and > + * %LANDLOCK_ACCESS_FS_WRITE_FILE. This access right is available since the > + * third version of the Landlock ABI. > * > * A directory can receive access rights related to files or directories. The > * following access right is applied to the directory itself, and the > @@ -139,10 +150,9 @@ struct landlock_path_beneath_attr { > * > * It is currently not possible to restrict some file-related actions > * accessible through these syscall families: :manpage:`chdir(2)`, > - * :manpage:`truncate(2)`, :manpage:`stat(2)`, :manpage:`flock(2)`, > - * :manpage:`chmod(2)`, :manpage:`chown(2)`, :manpage:`setxattr(2)`, > - * :manpage:`utime(2)`, :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, > - * :manpage:`access(2)`. > + * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`, > + * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`, > + * :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`. > * Future Landlock evolutions will enable to restrict them. > */ > /* clang-format off */ > @@ -160,6 +170,7 @@ struct landlock_path_beneath_attr { > #define LANDLOCK_ACCESS_FS_MAKE_BLOCK (1ULL << 11) > #define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12) > #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13) > +#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) > /* clang-format on */ > > #endif /* _UAPI_LINUX_LANDLOCK_H */ > diff --git a/security/landlock/fs.c b/security/landlock/fs.c > index 083dd3d359de..80d507ce2305 100644 > --- a/security/landlock/fs.c > +++ b/security/landlock/fs.c > @@ -146,7 +146,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode) > #define ACCESS_FILE ( \ > LANDLOCK_ACCESS_FS_EXECUTE | \ > LANDLOCK_ACCESS_FS_WRITE_FILE | \ > - LANDLOCK_ACCESS_FS_READ_FILE) > + LANDLOCK_ACCESS_FS_READ_FILE | \ > + LANDLOCK_ACCESS_FS_TRUNCATE) > /* clang-format on */ > > /* > @@ -297,6 +298,18 @@ get_handled_accesses(const struct landlock_ruleset *const domain) > return access_dom & LANDLOCK_MASK_ACCESS_FS; > } > > +/* > + * init_layer_masks - Populates @layer_masks such that for each access right in > + * @access_request, the bits for all the layers are set where this access right > + * is handled. Thanks for this extra documentation! Can you convert it to a proper code documentation (even if it not used yet), with a heading `/**` and a short title following the function name? Something like "init_layer_masks - Initialize layer masks". Please follow this convention for the other doc strings, or just use a paragraph in a simple comment (e.g. for get_required_file_open_access). Because there is no direct link with Landlock supporting truncation, this should be in a standalone patch, but you can keep it in this series. > + * > + * @domain: The domain that defines the current restrictions. > + * @access_request: The requested access rights to check. > + * @layer_masks: The layer masks to populate. > + * > + * Returns: An access mask where each access right bit is set which is handled > + * in any of the active layers in @domain. > + */ > static inline access_mask_t > init_layer_masks(const struct landlock_ruleset *const domain, > const access_mask_t access_request, > @@ -1141,9 +1154,19 @@ static int hook_path_rmdir(const struct path *const dir, > return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR); > } > > +static int hook_path_truncate(const struct path *const path) > +{ > + return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE); > +} > + > /* File hooks */ > > -static inline access_mask_t get_file_access(const struct file *const file) > +/* > + * get_required_file_open_access - Returns the access rights that are required > + * for opening the file, depending on the file type and open mode. > + */ > +static inline access_mask_t > +get_required_file_open_access(const struct file *const file) > { > access_mask_t access = 0; > > @@ -1163,17 +1186,82 @@ static inline access_mask_t get_file_access(const struct file *const file) > > static int hook_file_open(struct file *const file) > { > + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; > + access_mask_t open_access_request, full_access_request, allowed_access; > + const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE; > const struct landlock_ruleset *const dom = > landlock_get_current_domain(); > > - if (!dom) > + if (!dom) { > + /* > + * Grants all access rights, even if most of them are not > + * checked later on. It is more consistent. > + */ > + landlock_file(file)->allowed_access = LANDLOCK_MASK_ACCESS_FS; This looks like the right approach but unfortunately, because there is multiple ways to get a file descriptors (e.g. memfd_create, which is worth mentioning in a comment), this doesn't work well. For now, it only makes sense for Landlock to restrict file descriptors obtained through open(2). We can then move this initialization to a new hook implementation for file_alloc_security. I think this is the bug Nathan reported. We should have a test with memfd_create(2) to make sure it works as expected. I think the documentation is still correct though. > return 0; > + } > + > /* > - * Because a file may be opened with O_PATH, get_file_access() may > - * return 0. This case will be handled with a future Landlock > + * Because a file may be opened with O_PATH, get_required_file_open_access() > + * may return 0. This case will be handled with a future Landlock > * evolution. > */ > - return check_access_path(dom, &file->f_path, get_file_access(file)); > + open_access_request = get_required_file_open_access(file); > + > + /* > + * We look up more access than what we immediately need for open(), so > + * that we can later authorize operations on opened files. > + */ > + full_access_request = open_access_request | optional_access; > + > + allowed_access = full_access_request; > + if (!is_access_to_paths_allowed( > + dom, &file->f_path, > + init_layer_masks(dom, full_access_request, &layer_masks), > + &layer_masks, NULL, 0, NULL, NULL)) { I'd prefer (less error prone and easier to read) to add an is_access_paths_allowed branch to initialize allowed_access with full_access_request, and tweak this branch to initialize allowed_access with 0 and then populate it according to !layer_masks[access_bit]. > + unsigned long access_bit; > + unsigned long access_req = full_access_request; const unsigned long access_req > + > + /* > + * Calculate the actual allowed access rights from layer_masks. > + * Remove each access right from allowed_access which has been > + * vetoed by any layer. > + */ > + for_each_set_bit(access_bit, &access_req, > + ARRAY_SIZE(layer_masks)) { > + if (layer_masks[access_bit]) > + allowed_access &= ~BIT_ULL(access_bit); > + } > + } We can move `landlock_file(file)->allowed_access = allowed_access` here to be sure that the struct file allowed access is consistent even if it should not be used (because access may be denied). > + > + if (open_access_request & ~allowed_access) > + return -EACCES; And here invert the check ((open_access_request & allowed_access) == open_access_request) to make it more consistent with other checks… > + > + /* > + * For operations on already opened files (i.e. ftruncate()), it is the > + * access rights at the time of open() which decide whether the > + * operation is permitted. Therefore, we record the relevant subset of > + * file access rights in the opened struct file. > + */ > + landlock_file(file)->allowed_access = allowed_access; > + return 0; …and return -EACCES here. > +} > + > +static int hook_file_truncate(struct file *const file) > +{ > + /* > + * Allows truncation if the truncate right was available at the time of > + * opening the file, to get a consistent access check as for read, write > + * and execute operations. > + * > + * Note: For checks done based on the file's Landlock rights, we enforce s/file's Landlock rights/file's Landlock allowed access/ maybe? > + * them independently of whether the current thread is in a Landlock > + * domain, so that open files passed between independent processes > + * retain their behaviour. > + */ > + if (landlock_file(file)->allowed_access & LANDLOCK_ACCESS_FS_TRUNCATE) > + return 0; > + return -EACCES; > } > > static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { > @@ -1193,6 +1281,8 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { > LSM_HOOK_INIT(path_symlink, hook_path_symlink), > LSM_HOOK_INIT(path_unlink, hook_path_unlink), > LSM_HOOK_INIT(path_rmdir, hook_path_rmdir), > + LSM_HOOK_INIT(path_truncate, hook_path_truncate), > + LSM_HOOK_INIT(file_truncate, hook_file_truncate), Please move the hook_file_truncate entry after the hook_file_open one, these entries are in the same order as their hook implementations. > > LSM_HOOK_INIT(file_open, hook_file_open), > }; > diff --git a/security/landlock/fs.h b/security/landlock/fs.h > index 8db7acf9109b..488e4813680a 100644 > --- a/security/landlock/fs.h > +++ b/security/landlock/fs.h > @@ -36,6 +36,24 @@ struct landlock_inode_security { > struct landlock_object __rcu *object; > }; > > +/** > + * struct landlock_file_security - File security blob > + * > + * This information is populated when opening a file in hook_file_open, and > + * tracks the relevant Landlock access rights that were available at the time > + * of opening the file. Other LSM hooks use these rights in order to authorize > + * operations on already opened files. > + */ > +struct landlock_file_security { > + /** > + * @allowed_access: Access rights that were available at the time of > + * opening the file. This is not necessarily the full set of access > + * rights available at that time, but it's the necessary subset as > + * needed to authorize later operations on the open file. > + */ > + access_mask_t allowed_access; > +}; > + > /** > * struct landlock_superblock_security - Superblock security blob > * > @@ -50,6 +68,12 @@ struct landlock_superblock_security { > atomic_long_t inode_refs; > }; > > +static inline struct landlock_file_security * > +landlock_file(const struct file *const file) > +{ > + return file->f_security + landlock_blob_sizes.lbs_file; > +} > + > static inline struct landlock_inode_security * > landlock_inode(const struct inode *const inode) > { > diff --git a/security/landlock/limits.h b/security/landlock/limits.h > index b54184ab9439..82288f0e9e5e 100644 > --- a/security/landlock/limits.h > +++ b/security/landlock/limits.h > @@ -18,7 +18,7 @@ > #define LANDLOCK_MAX_NUM_LAYERS 16 > #define LANDLOCK_MAX_NUM_RULES U32_MAX > > -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_REFER > +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_TRUNCATE > #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1) > #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) > > diff --git a/security/landlock/setup.c b/security/landlock/setup.c > index f8e8e980454c..3f196d2ce4f9 100644 > --- a/security/landlock/setup.c > +++ b/security/landlock/setup.c > @@ -19,6 +19,7 @@ bool landlock_initialized __lsm_ro_after_init = false; > > struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = { > .lbs_cred = sizeof(struct landlock_cred_security), > + .lbs_file = sizeof(struct landlock_file_security), > .lbs_inode = sizeof(struct landlock_inode_security), > .lbs_superblock = sizeof(struct landlock_superblock_security), > }; > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c > index 735a0865ea11..f4d6fc7ed17f 100644 > --- a/security/landlock/syscalls.c > +++ b/security/landlock/syscalls.c > @@ -129,7 +129,7 @@ static const struct file_operations ruleset_fops = { > .write = fop_dummy_write, > }; > > -#define LANDLOCK_ABI_VERSION 2 > +#define LANDLOCK_ABI_VERSION 3 > > /** > * sys_landlock_create_ruleset - Create a new ruleset > diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c > index da9290817866..72cdae277b02 100644 > --- a/tools/testing/selftests/landlock/base_test.c > +++ b/tools/testing/selftests/landlock/base_test.c > @@ -75,7 +75,7 @@ TEST(abi_version) > const struct landlock_ruleset_attr ruleset_attr = { > .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE, > }; > - ASSERT_EQ(2, landlock_create_ruleset(NULL, 0, > + ASSERT_EQ(3, landlock_create_ruleset(NULL, 0, > LANDLOCK_CREATE_RULESET_VERSION)); > > ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0, > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > index 45de42a027c5..87b28d14a1aa 100644 > --- a/tools/testing/selftests/landlock/fs_test.c > +++ b/tools/testing/selftests/landlock/fs_test.c > @@ -406,9 +406,10 @@ TEST_F_FORK(layout1, inval) > #define ACCESS_FILE ( \ > LANDLOCK_ACCESS_FS_EXECUTE | \ > LANDLOCK_ACCESS_FS_WRITE_FILE | \ > - LANDLOCK_ACCESS_FS_READ_FILE) > + LANDLOCK_ACCESS_FS_READ_FILE | \ > + LANDLOCK_ACCESS_FS_TRUNCATE) > > -#define ACCESS_LAST LANDLOCK_ACCESS_FS_REFER > +#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE > > #define ACCESS_ALL ( \ > ACCESS_FILE | \ > @@ -422,7 +423,7 @@ TEST_F_FORK(layout1, inval) > LANDLOCK_ACCESS_FS_MAKE_FIFO | \ > LANDLOCK_ACCESS_FS_MAKE_BLOCK | \ > LANDLOCK_ACCESS_FS_MAKE_SYM | \ > - ACCESS_LAST) > + LANDLOCK_ACCESS_FS_REFER) > > /* clang-format on */ >
On Wed, Oct 05, 2022 at 08:52:41PM +0200, Mickaël Salaün wrote: > On 04/10/2022 21:56, Nathan Chancellor wrote: > > Hi Günther, > > > > On Sat, Oct 01, 2022 at 05:49:03PM +0200, Günther Noack wrote: > > > Introduce the LANDLOCK_ACCESS_FS_TRUNCATE flag for file truncation. > > > > > > This flag hooks into the path_truncate LSM hook and covers file > > > truncation using truncate(2), ftruncate(2), open(2) with O_TRUNC, as > > > well as creat(). > > > > > > This change also increments the Landlock ABI version, updates > > > corresponding selftests, and updates code documentation to document > > > the flag. > > > > > > The following operations are restricted: > > > > > > open(): requires the LANDLOCK_ACCESS_FS_TRUNCATE right if a file gets > > > implicitly truncated as part of the open() (e.g. using O_TRUNC). > > > > > > Notable special cases: > > > * open(..., O_RDONLY|O_TRUNC) can truncate files as well in Linux > > > * open() with O_TRUNC does *not* need the TRUNCATE right when it > > > creates a new file. > > > > > > truncate() (on a path): requires the LANDLOCK_ACCESS_FS_TRUNCATE > > > right. > > > > > > ftruncate() (on a file): requires that the file had the TRUNCATE right > > > when it was previously opened. > > > > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com> > > > > I just bisected a crash in QEMU with Debian's arm64 configuration to > > this change in -next as commit b40deebe7679 ("landlock: Support file > > truncation"), which I was able to reproduce like so: > > Thanks for the report Nathan. I've found an issue in this patch and fixed it > in -next with this (rebased) commit: > https://git.kernel.org/mic/c/b40deebe7679b05d4852488ef531e189a9621f2e > You should already have this update since I pushed it Monday. > Please let us know if this fixed the issue. I'm afraid Nathan already tested it with the version from the mic -next branch b40deebe7679 ("landlock: Support file truncation") Nathan, thank you for the report and especially for the detailed step by step instructions! I have tried to reproduce it with the steps you suggested with the root file system you linked to, but I'm afraid I was unable to reproduce the crash. When I've tried it, the kernel boots up to init and shuts down again, but does not run into the issue you're reporting: ... [ 1.165628] Run /init as init process Starting syslogd: OK Starting klogd: OK Running sysctl: OK Saving random seed: OK Starting network: OK Linux version 6.0.0-rc7-00004-gb40deebe7679 (gnoack@nuc) (aarch64-linux-gnu-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 2.39) #4 SMP PREEMPT Thu Oct 6 21:45:59 CEST 2022 Stopping network: OK Saving random seed: OK Stopping klogd: OK Stopping syslogd: OK umount: devtmpfs busy - remounted read-only umount: can't unmount /: Invalid argument The system is going down NOW! Sent SIGTERM to all processes Sent SIGKILL to all processes Requesting system poweroff [ 5.303758] kvm: exiting hardware virtualization [ 5.315878] Flash device refused suspend due to active operation (state 20) [ 5.318164] Flash device refused suspend due to active operation (state 20) [ 5.322274] reboot: Power down I've been compiling the kernel using make -j8 ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build olddefconfig Image.gz and started the emulator using qemu-system-aarch64 -machine virt,gic-version=max,virtualization=true \ -cpu max,pauth-impdef=true \ -kernel /home/gnoack/linux/build/arch/arm64/boot/Image.gz \ -append "console=ttyAMA0 earlycon" \ -display none -m 512m -nodefaults -no-reboot -serial mon:stdio \ -initrd /tmp/rootfs.cpio I have attempted it with both the commit from the mic -next branch, as well as with my own client that is still lacking Mickaël's LSM hook fix. The commits I've tried are: * b40deebe7679 ("landlock: Support file truncation") * 224e66a32f16 ("landlock: Document Landlock's file truncation support") I've built the kernel from scratch from the config file you suggested, to be sure. I used the rootfs.cpio file you provided from Github, and gcc 12.2.0 and qemu 7.1.0 from the Arch Linux repositories. At this point, I don't know what else I can try to get it to crash... it seems to work fine for me. —Günther > > > > $ mkdir -p build/deb > > > > $ cd build/deb > > > > $ curl -LSsO http://ftp.us.debian.org/debian/pool/main/l/linux-signed-arm64/linux-image-5.19.0-2-arm64_5.19.11-1_arm64.deb > > > > $ ar x linux-image-5.19.0-2-arm64_5.19.11-1_arm64.deb > > > > $ tar xJf data.tar.xz > > > > $ cp boot/config-5.19.0-2-arm64 ../.config > > > > $ cd ../.. > > > > $ make -skj"$(nproc)" ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- O=build olddefconfig Image.gz > > > > $ qemu-system-aarch64 \ > > -machine virt,gic-version=max,virtualization=true \ > > -cpu max,pauth-impdef=true \ > > -kernel build/arch/arm64/boot/Image.gz \ > > -append "console=ttyAMA0 earlycon" \ > > -display none \ > > -initrd .../rootfs.cpio \ > > -m 512m \ > > -nodefaults \ > > -no-reboot \ > > -serial mon:stdio > > ... > > [ 0.000000] Linux version 6.0.0-rc7+ (nathan@dev-arch.thelio-3990X) (aarch64-linux-gnu-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 2.39) #1 SMP Tue Oct 4 12:48:50 MST 2022 > > ... > > [ 0.518570] Unable to handle kernel paging request at virtual address ffff00000851ff8a > > [ 0.518785] Mem abort info: > > [ 0.518867] ESR = 0x0000000097c0c061 > > [ 0.519001] EC = 0x25: DABT (current EL), IL = 32 bits > > [ 0.519155] SET = 0, FnV = 0 > > [ 0.519267] EA = 0, S1PTW = 0 > > [ 0.519386] FSC = 0x21: alignment fault > > [ 0.519524] Data abort info: > > [ 0.519615] Access size = 8 byte(s) > > [ 0.519722] SSE = 0, SRT = 0 > > [ 0.519817] SF = 1, AR = 1 > > [ 0.519920] CM = 0, WnR = 1 > > [ 0.520040] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000041711000 > > [ 0.520225] [ffff00000851ff8a] pgd=180000005fff8003, p4d=180000005fff8003, pud=180000005fff7003, pmd=180000005ffbd003, pte=006800004851ff07 > > [ 0.521121] Internal error: Oops: 97c0c061 [#1] SMP > > [ 0.521364] Modules linked in: > > [ 0.521592] CPU: 0 PID: 9 Comm: kworker/u2:0 Not tainted 6.0.0-rc7+ #1 > > [ 0.521863] Hardware name: linux,dummy-virt (DT) > > [ 0.522325] Workqueue: events_unbound async_run_entry_fn > > [ 0.522973] pstate: 60400009 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) > > [ 0.523193] pc : apparmor_file_alloc_security+0x98/0x1e0 > > [ 0.523431] lr : apparmor_file_alloc_security+0x48/0x1e0 > > [ 0.523594] sp : ffff800008093960 > > [ 0.523708] x29: ffff800008093960 x28: ffff800008093b30 x27: ffff000002602600 > > [ 0.523978] x26: ffffd79796ecf8c0 x25: ffff00000241e705 x24: ffffd79797d98068 > > [ 0.524199] x23: ffff00000851ff82 x22: ffff00000851ff80 x21: 0000000000000002 > > [ 0.524431] x20: ffffd79796ff5000 x19: ffff00000241ceb0 x18: ffffffffffffffff > > [ 0.524647] x17: 000000000000003f x16: ffffd79797678008 x15: 0000000000000000 > > [ 0.524850] x14: 0000000000000001 x13: 0000000000000000 x12: 0000000000000006 > > [ 0.525087] x11: ffff00001feef940 x10: ffffd7979768f8a0 x9 : ffffd79796c1e51c > > [ 0.525325] x8 : ffff00000851ffa0 x7 : 0000000000000000 x6 : 0000000000001e0b > > [ 0.525531] x5 : ffff00000851ff80 x4 : ffff800008093990 x3 : ffff000002419700 > > [ 0.525745] x2 : 0000000000000001 x1 : ffff00000851ff8a x0 : ffff00000241ceb0 > > [ 0.526034] Call trace: > > [ 0.526166] apparmor_file_alloc_security+0x98/0x1e0 > > [ 0.526424] security_file_alloc+0x6c/0xf0 > > [ 0.526570] __alloc_file+0x5c/0xf0 > > [ 0.526699] alloc_empty_file+0x68/0x10c > > [ 0.526816] path_openat+0x50/0x106c > > [ 0.526929] do_filp_open+0x88/0x13c > > [ 0.527041] filp_open+0x110/0x1b0 > > [ 0.527143] do_name+0xbc/0x230 > > [ 0.527256] write_buffer+0x40/0x60 > > [ 0.527359] unpack_to_rootfs+0x100/0x2bc > > [ 0.527479] do_populate_rootfs+0x70/0x134 > > [ 0.527602] async_run_entry_fn+0x40/0x1c0 > > [ 0.527723] process_one_work+0x1f4/0x450 > > [ 0.527851] worker_thread+0x188/0x4c0 > > [ 0.527980] kthread+0xe0/0xe4 > > [ 0.528066] ret_from_fork+0x10/0x20 > > [ 0.528317] Code: 52800002 d2800000 d2800013 910022e1 (c89ffc20) > > [ 0.528736] ---[ end trace 0000000000000000 ]--- > > ... > > > > A rootfs is available at [1] but I don't think it should be necessary > > for reproducing this. If there is any additional information I can > > provide or patches I can test, I am more than happy to do so! > > > > [1]: https://github.com/ClangBuiltLinux/boot-utils/raw/bf2fd3500d87f78a914bfc3769b2240f5632e5b9/images/arm64/rootfs.cpio.zst > > > > Cheers, > > Nathan --
On Wed, Oct 05, 2022 at 08:55:42PM +0200, Mickaël Salaün wrote: > > On 01/10/2022 17:49, Günther Noack wrote: > > Introduce the LANDLOCK_ACCESS_FS_TRUNCATE flag for file truncation. > > > > This flag hooks into the path_truncate LSM hook and covers file > > truncation using truncate(2), ftruncate(2), open(2) with O_TRUNC, as > > well as creat(). > > > > This change also increments the Landlock ABI version, updates > > corresponding selftests, and updates code documentation to document > > the flag. > > > > The following operations are restricted: > > > > open(): requires the LANDLOCK_ACCESS_FS_TRUNCATE right if a file gets > > implicitly truncated as part of the open() (e.g. using O_TRUNC). > > > > Notable special cases: > > * open(..., O_RDONLY|O_TRUNC) can truncate files as well in Linux > > * open() with O_TRUNC does *not* need the TRUNCATE right when it > > creates a new file. > > > > truncate() (on a path): requires the LANDLOCK_ACCESS_FS_TRUNCATE > > right. > > > > ftruncate() (on a file): requires that the file had the TRUNCATE right > > when it was previously opened. > > > > Signed-off-by: Günther Noack <gnoack3000@gmail.com> > > --- > > include/uapi/linux/landlock.h | 21 +++- > > security/landlock/fs.c | 102 +++++++++++++++++-- > > security/landlock/fs.h | 24 +++++ > > security/landlock/limits.h | 2 +- > > security/landlock/setup.c | 1 + > > security/landlock/syscalls.c | 2 +- > > tools/testing/selftests/landlock/base_test.c | 2 +- > > tools/testing/selftests/landlock/fs_test.c | 7 +- > > 8 files changed, 144 insertions(+), 17 deletions(-) > > > > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h > > index 23df4e0e8ace..d830cdfdbe56 100644 > > --- a/include/uapi/linux/landlock.h > > +++ b/include/uapi/linux/landlock.h > > @@ -95,8 +95,19 @@ struct landlock_path_beneath_attr { > > * A file can only receive these access rights: > > * > > * - %LANDLOCK_ACCESS_FS_EXECUTE: Execute a file. > > - * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. > > + * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. Note that > > + * you might additionally need the `LANDLOCK_ACCESS_FS_TRUNCATE` right in > > %LANDLOCK_ACCESS_FS_TRUNCATE Done. > > + * order to overwrite files with :manpage:`open(2)` using `O_TRUNC` or > > + * :manpage:`creat(2)`. > > * - %LANDLOCK_ACCESS_FS_READ_FILE: Open a file with read access. > > + * - %LANDLOCK_ACCESS_FS_TRUNCATE: Truncate a file with :manpage:`truncate(2)`, > > + * :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with > > + * `O_TRUNC`. Whether an opened file can be truncated with > > %O_TRUNC Done. (Also in the case a few line above.) > > + * :manpage:`ftruncate(2)` is determined during :manpage:`open(2)`, in the > > + * same way as read and write permissions are checked during > > + * :manpage:`open(2)` using %LANDLOCK_ACCESS_FS_READ_FILE and > > + * %LANDLOCK_ACCESS_FS_WRITE_FILE. This access right is available since the > > + * third version of the Landlock ABI. > > * > > * A directory can receive access rights related to files or directories. The > > * following access right is applied to the directory itself, and the > > @@ -139,10 +150,9 @@ struct landlock_path_beneath_attr { > > * > > * It is currently not possible to restrict some file-related actions > > * accessible through these syscall families: :manpage:`chdir(2)`, > > - * :manpage:`truncate(2)`, :manpage:`stat(2)`, :manpage:`flock(2)`, > > - * :manpage:`chmod(2)`, :manpage:`chown(2)`, :manpage:`setxattr(2)`, > > - * :manpage:`utime(2)`, :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, > > - * :manpage:`access(2)`. > > + * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`, > > + * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`, > > + * :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`. > > * Future Landlock evolutions will enable to restrict them. > > */ > > /* clang-format off */ > > @@ -160,6 +170,7 @@ struct landlock_path_beneath_attr { > > #define LANDLOCK_ACCESS_FS_MAKE_BLOCK (1ULL << 11) > > #define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12) > > #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13) > > +#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) > > /* clang-format on */ > > #endif /* _UAPI_LINUX_LANDLOCK_H */ > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c > > index 083dd3d359de..80d507ce2305 100644 > > --- a/security/landlock/fs.c > > +++ b/security/landlock/fs.c > > @@ -146,7 +146,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode) > > #define ACCESS_FILE ( \ > > LANDLOCK_ACCESS_FS_EXECUTE | \ > > LANDLOCK_ACCESS_FS_WRITE_FILE | \ > > - LANDLOCK_ACCESS_FS_READ_FILE) > > + LANDLOCK_ACCESS_FS_READ_FILE | \ > > + LANDLOCK_ACCESS_FS_TRUNCATE) > > /* clang-format on */ > > /* > > @@ -297,6 +298,18 @@ get_handled_accesses(const struct landlock_ruleset *const domain) > > return access_dom & LANDLOCK_MASK_ACCESS_FS; > > } > > +/* > > + * init_layer_masks - Populates @layer_masks such that for each access right in > > + * @access_request, the bits for all the layers are set where this access right > > + * is handled. > > Thanks for this extra documentation! > > Can you convert it to a proper code documentation (even if it not used yet), > with a heading `/**` and a short title following the function name? > Something like "init_layer_masks - Initialize layer masks". Please follow > this convention for the other doc strings, or just use a paragraph in a > simple comment (e.g. for get_required_file_open_access). > > Because there is no direct link with Landlock supporting truncation, this > should be in a standalone patch, but you can keep it in this series. Done, I split it off into a separate patch and fixed the formatting to use /** and the right documentation format. I fixed up the get_required_file_open_access documentation as well, but kept it in this patch, because it's a related change. > > + * > > + * @domain: The domain that defines the current restrictions. > > + * @access_request: The requested access rights to check. > > + * @layer_masks: The layer masks to populate. > > + * > > + * Returns: An access mask where each access right bit is set which is handled > > + * in any of the active layers in @domain. > > + */ > > static inline access_mask_t > > init_layer_masks(const struct landlock_ruleset *const domain, > > const access_mask_t access_request, > > @@ -1141,9 +1154,19 @@ static int hook_path_rmdir(const struct path *const dir, > > return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR); > > } > > +static int hook_path_truncate(const struct path *const path) > > +{ > > + return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE); > > +} > > + > > /* File hooks */ > > -static inline access_mask_t get_file_access(const struct file *const file) > > +/* > > + * get_required_file_open_access - Returns the access rights that are required > > + * for opening the file, depending on the file type and open mode. > > + */ > > +static inline access_mask_t > > +get_required_file_open_access(const struct file *const file) > > { > > access_mask_t access = 0; > > @@ -1163,17 +1186,82 @@ static inline access_mask_t get_file_access(const struct file *const file) > > static int hook_file_open(struct file *const file) > > { > > + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; > > + access_mask_t open_access_request, full_access_request, allowed_access; > > + const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE; > > const struct landlock_ruleset *const dom = > > landlock_get_current_domain(); > > - if (!dom) > > + if (!dom) { > > + /* > > + * Grants all access rights, even if most of them are not > > + * checked later on. It is more consistent. > > + */ > > + landlock_file(file)->allowed_access = LANDLOCK_MASK_ACCESS_FS; > > This looks like the right approach but unfortunately, because there is > multiple ways to get a file descriptors (e.g. memfd_create, which is worth > mentioning in a comment), this doesn't work well. For now, it only makes > sense for Landlock to restrict file descriptors obtained through open(2). We > can then move this initialization to a new hook implementation for > file_alloc_security. Good catch -- that was indeed still a bug. > I think this is the bug Nathan reported. As discussed on the other thread, that is unclear, and I failed to reproduce his crash here. He did try it with the fixed version from your next branch. > We should have a test with memfd_create(2) to make sure it works as > expected. I think the documentation is still correct though. I added the memfd_create(2) test in a separate commit, verified that it fails with the old version of the code, as expected, and fixed the bug in this commit with the same approach which you also took on your next branch, by implementing a file_alloc_security hook. > > return 0; > > + } > > + > > /* > > - * Because a file may be opened with O_PATH, get_file_access() may > > - * return 0. This case will be handled with a future Landlock > > + * Because a file may be opened with O_PATH, get_required_file_open_access() > > + * may return 0. This case will be handled with a future Landlock > > * evolution. > > */ > > - return check_access_path(dom, &file->f_path, get_file_access(file)); > > + open_access_request = get_required_file_open_access(file); > > + > > + /* > > + * We look up more access than what we immediately need for open(), so > > + * that we can later authorize operations on opened files. > > + */ > > + full_access_request = open_access_request | optional_access; > > + > > + allowed_access = full_access_request; > > + if (!is_access_to_paths_allowed( > > + dom, &file->f_path, > > + init_layer_masks(dom, full_access_request, &layer_masks), > > + &layer_masks, NULL, 0, NULL, NULL)) { > > I'd prefer (less error prone and easier to read) to add an > is_access_paths_allowed branch to initialize allowed_access with > full_access_request, and tweak this branch to initialize allowed_access with > 0 and then populate it according to !layer_masks[access_bit]. Done, this makes sense. > > + unsigned long access_bit; > > + unsigned long access_req = full_access_request; > > const unsigned long access_req Done. > > + > > + /* > > + * Calculate the actual allowed access rights from layer_masks. > > + * Remove each access right from allowed_access which has been > > + * vetoed by any layer. > > + */ > > + for_each_set_bit(access_bit, &access_req, > > + ARRAY_SIZE(layer_masks)) { > > + if (layer_masks[access_bit]) > > + allowed_access &= ~BIT_ULL(access_bit); > + } > > + } > > We can move `landlock_file(file)->allowed_access = allowed_access` here to > be sure that the struct file allowed access is consistent even if it should > not be used (because access may be denied). Done, moved it after the "if" branch to clarify that it happens unconditionally. > > + > > + if (open_access_request & ~allowed_access) > > + return -EACCES; > > And here invert the check ((open_access_request & allowed_access) == > open_access_request) to make it more consistent with other checks… Done, good suggestion. I find this easier to read as well. > > + > > + /* > > + * For operations on already opened files (i.e. ftruncate()), it is the > > + * access rights at the time of open() which decide whether the > > + * operation is permitted. Therefore, we record the relevant subset of > > + * file access rights in the opened struct file. > > + */ > > + landlock_file(file)->allowed_access = allowed_access; > > + return 0; > > …and return -EACCES here. Done. > > +} > > + > > +static int hook_file_truncate(struct file *const file) > > +{ > > + /* > > + * Allows truncation if the truncate right was available at the time of > > + * opening the file, to get a consistent access check as for read, write > > + * and execute operations. > > + * > > + * Note: For checks done based on the file's Landlock rights, we enforce > > s/file's Landlock rights/file's Landlock allowed access/ maybe? Done. > > + * them independently of whether the current thread is in a Landlock > > + * domain, so that open files passed between independent processes > > + * retain their behaviour. > > + */ > > + if (landlock_file(file)->allowed_access & LANDLOCK_ACCESS_FS_TRUNCATE) > > + return 0; > > + return -EACCES; > > } > > static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { > > @@ -1193,6 +1281,8 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { > > LSM_HOOK_INIT(path_symlink, hook_path_symlink), > > LSM_HOOK_INIT(path_unlink, hook_path_unlink), > > LSM_HOOK_INIT(path_rmdir, hook_path_rmdir), > > + LSM_HOOK_INIT(path_truncate, hook_path_truncate), > > + LSM_HOOK_INIT(file_truncate, hook_file_truncate), > > Please move the hook_file_truncate entry after the hook_file_open one, these > entries are in the same order as their hook implementations. Done. > > LSM_HOOK_INIT(file_open, hook_file_open), > > }; > > diff --git a/security/landlock/fs.h b/security/landlock/fs.h > > index 8db7acf9109b..488e4813680a 100644 > > --- a/security/landlock/fs.h > > +++ b/security/landlock/fs.h > > @@ -36,6 +36,24 @@ struct landlock_inode_security { > > struct landlock_object __rcu *object; > > }; > > +/** > > + * struct landlock_file_security - File security blob > > + * > > + * This information is populated when opening a file in hook_file_open, and > > + * tracks the relevant Landlock access rights that were available at the time > > + * of opening the file. Other LSM hooks use these rights in order to authorize > > + * operations on already opened files. > > + */ > > +struct landlock_file_security { > > + /** > > + * @allowed_access: Access rights that were available at the time of > > + * opening the file. This is not necessarily the full set of access > > + * rights available at that time, but it's the necessary subset as > > + * needed to authorize later operations on the open file. > > + */ > > + access_mask_t allowed_access; > > +}; > > + > > /** > > * struct landlock_superblock_security - Superblock security blob > > * > > @@ -50,6 +68,12 @@ struct landlock_superblock_security { > > atomic_long_t inode_refs; > > }; > > +static inline struct landlock_file_security * > > +landlock_file(const struct file *const file) > > +{ > > + return file->f_security + landlock_blob_sizes.lbs_file; > > +} > > + > > static inline struct landlock_inode_security * > > landlock_inode(const struct inode *const inode) > > { > > diff --git a/security/landlock/limits.h b/security/landlock/limits.h > > index b54184ab9439..82288f0e9e5e 100644 > > --- a/security/landlock/limits.h > > +++ b/security/landlock/limits.h > > @@ -18,7 +18,7 @@ > > #define LANDLOCK_MAX_NUM_LAYERS 16 > > #define LANDLOCK_MAX_NUM_RULES U32_MAX > > -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_REFER > > +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_TRUNCATE > > #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1) > > #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) > > diff --git a/security/landlock/setup.c b/security/landlock/setup.c > > index f8e8e980454c..3f196d2ce4f9 100644 > > --- a/security/landlock/setup.c > > +++ b/security/landlock/setup.c > > @@ -19,6 +19,7 @@ bool landlock_initialized __lsm_ro_after_init = false; > > struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = { > > .lbs_cred = sizeof(struct landlock_cred_security), > > + .lbs_file = sizeof(struct landlock_file_security), > > .lbs_inode = sizeof(struct landlock_inode_security), > > .lbs_superblock = sizeof(struct landlock_superblock_security), > > }; > > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c > > index 735a0865ea11..f4d6fc7ed17f 100644 > > --- a/security/landlock/syscalls.c > > +++ b/security/landlock/syscalls.c > > @@ -129,7 +129,7 @@ static const struct file_operations ruleset_fops = { > > .write = fop_dummy_write, > > }; > > -#define LANDLOCK_ABI_VERSION 2 > > +#define LANDLOCK_ABI_VERSION 3 > > /** > > * sys_landlock_create_ruleset - Create a new ruleset > > diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c > > index da9290817866..72cdae277b02 100644 > > --- a/tools/testing/selftests/landlock/base_test.c > > +++ b/tools/testing/selftests/landlock/base_test.c > > @@ -75,7 +75,7 @@ TEST(abi_version) > > const struct landlock_ruleset_attr ruleset_attr = { > > .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE, > > }; > > - ASSERT_EQ(2, landlock_create_ruleset(NULL, 0, > > + ASSERT_EQ(3, landlock_create_ruleset(NULL, 0, > > LANDLOCK_CREATE_RULESET_VERSION)); > > ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0, > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > > index 45de42a027c5..87b28d14a1aa 100644 > > --- a/tools/testing/selftests/landlock/fs_test.c > > +++ b/tools/testing/selftests/landlock/fs_test.c > > @@ -406,9 +406,10 @@ TEST_F_FORK(layout1, inval) > > #define ACCESS_FILE ( \ > > LANDLOCK_ACCESS_FS_EXECUTE | \ > > LANDLOCK_ACCESS_FS_WRITE_FILE | \ > > - LANDLOCK_ACCESS_FS_READ_FILE) > > + LANDLOCK_ACCESS_FS_READ_FILE | \ > > + LANDLOCK_ACCESS_FS_TRUNCATE) > > -#define ACCESS_LAST LANDLOCK_ACCESS_FS_REFER > > +#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE > > #define ACCESS_ALL ( \ > > ACCESS_FILE | \ > > @@ -422,7 +423,7 @@ TEST_F_FORK(layout1, inval) > > LANDLOCK_ACCESS_FS_MAKE_FIFO | \ > > LANDLOCK_ACCESS_FS_MAKE_BLOCK | \ > > LANDLOCK_ACCESS_FS_MAKE_SYM | \ > > - ACCESS_LAST) > > + LANDLOCK_ACCESS_FS_REFER) > > /* clang-format on */ --
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h index 23df4e0e8ace..d830cdfdbe56 100644 --- a/include/uapi/linux/landlock.h +++ b/include/uapi/linux/landlock.h @@ -95,8 +95,19 @@ struct landlock_path_beneath_attr { * A file can only receive these access rights: * * - %LANDLOCK_ACCESS_FS_EXECUTE: Execute a file. - * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. + * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Open a file with write access. Note that + * you might additionally need the `LANDLOCK_ACCESS_FS_TRUNCATE` right in + * order to overwrite files with :manpage:`open(2)` using `O_TRUNC` or + * :manpage:`creat(2)`. * - %LANDLOCK_ACCESS_FS_READ_FILE: Open a file with read access. + * - %LANDLOCK_ACCESS_FS_TRUNCATE: Truncate a file with :manpage:`truncate(2)`, + * :manpage:`ftruncate(2)`, :manpage:`creat(2)`, or :manpage:`open(2)` with + * `O_TRUNC`. Whether an opened file can be truncated with + * :manpage:`ftruncate(2)` is determined during :manpage:`open(2)`, in the + * same way as read and write permissions are checked during + * :manpage:`open(2)` using %LANDLOCK_ACCESS_FS_READ_FILE and + * %LANDLOCK_ACCESS_FS_WRITE_FILE. This access right is available since the + * third version of the Landlock ABI. * * A directory can receive access rights related to files or directories. The * following access right is applied to the directory itself, and the @@ -139,10 +150,9 @@ struct landlock_path_beneath_attr { * * It is currently not possible to restrict some file-related actions * accessible through these syscall families: :manpage:`chdir(2)`, - * :manpage:`truncate(2)`, :manpage:`stat(2)`, :manpage:`flock(2)`, - * :manpage:`chmod(2)`, :manpage:`chown(2)`, :manpage:`setxattr(2)`, - * :manpage:`utime(2)`, :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, - * :manpage:`access(2)`. + * :manpage:`stat(2)`, :manpage:`flock(2)`, :manpage:`chmod(2)`, + * :manpage:`chown(2)`, :manpage:`setxattr(2)`, :manpage:`utime(2)`, + * :manpage:`ioctl(2)`, :manpage:`fcntl(2)`, :manpage:`access(2)`. * Future Landlock evolutions will enable to restrict them. */ /* clang-format off */ @@ -160,6 +170,7 @@ struct landlock_path_beneath_attr { #define LANDLOCK_ACCESS_FS_MAKE_BLOCK (1ULL << 11) #define LANDLOCK_ACCESS_FS_MAKE_SYM (1ULL << 12) #define LANDLOCK_ACCESS_FS_REFER (1ULL << 13) +#define LANDLOCK_ACCESS_FS_TRUNCATE (1ULL << 14) /* clang-format on */ #endif /* _UAPI_LINUX_LANDLOCK_H */ diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 083dd3d359de..80d507ce2305 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -146,7 +146,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode) #define ACCESS_FILE ( \ LANDLOCK_ACCESS_FS_EXECUTE | \ LANDLOCK_ACCESS_FS_WRITE_FILE | \ - LANDLOCK_ACCESS_FS_READ_FILE) + LANDLOCK_ACCESS_FS_READ_FILE | \ + LANDLOCK_ACCESS_FS_TRUNCATE) /* clang-format on */ /* @@ -297,6 +298,18 @@ get_handled_accesses(const struct landlock_ruleset *const domain) return access_dom & LANDLOCK_MASK_ACCESS_FS; } +/* + * init_layer_masks - Populates @layer_masks such that for each access right in + * @access_request, the bits for all the layers are set where this access right + * is handled. + * + * @domain: The domain that defines the current restrictions. + * @access_request: The requested access rights to check. + * @layer_masks: The layer masks to populate. + * + * Returns: An access mask where each access right bit is set which is handled + * in any of the active layers in @domain. + */ static inline access_mask_t init_layer_masks(const struct landlock_ruleset *const domain, const access_mask_t access_request, @@ -1141,9 +1154,19 @@ static int hook_path_rmdir(const struct path *const dir, return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR); } +static int hook_path_truncate(const struct path *const path) +{ + return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE); +} + /* File hooks */ -static inline access_mask_t get_file_access(const struct file *const file) +/* + * get_required_file_open_access - Returns the access rights that are required + * for opening the file, depending on the file type and open mode. + */ +static inline access_mask_t +get_required_file_open_access(const struct file *const file) { access_mask_t access = 0; @@ -1163,17 +1186,82 @@ static inline access_mask_t get_file_access(const struct file *const file) static int hook_file_open(struct file *const file) { + layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS] = {}; + access_mask_t open_access_request, full_access_request, allowed_access; + const access_mask_t optional_access = LANDLOCK_ACCESS_FS_TRUNCATE; const struct landlock_ruleset *const dom = landlock_get_current_domain(); - if (!dom) + if (!dom) { + /* + * Grants all access rights, even if most of them are not + * checked later on. It is more consistent. + */ + landlock_file(file)->allowed_access = LANDLOCK_MASK_ACCESS_FS; return 0; + } + /* - * Because a file may be opened with O_PATH, get_file_access() may - * return 0. This case will be handled with a future Landlock + * Because a file may be opened with O_PATH, get_required_file_open_access() + * may return 0. This case will be handled with a future Landlock * evolution. */ - return check_access_path(dom, &file->f_path, get_file_access(file)); + open_access_request = get_required_file_open_access(file); + + /* + * We look up more access than what we immediately need for open(), so + * that we can later authorize operations on opened files. + */ + full_access_request = open_access_request | optional_access; + + allowed_access = full_access_request; + if (!is_access_to_paths_allowed( + dom, &file->f_path, + init_layer_masks(dom, full_access_request, &layer_masks), + &layer_masks, NULL, 0, NULL, NULL)) { + unsigned long access_bit; + unsigned long access_req = full_access_request; + + /* + * Calculate the actual allowed access rights from layer_masks. + * Remove each access right from allowed_access which has been + * vetoed by any layer. + */ + for_each_set_bit(access_bit, &access_req, + ARRAY_SIZE(layer_masks)) { + if (layer_masks[access_bit]) + allowed_access &= ~BIT_ULL(access_bit); + } + } + + if (open_access_request & ~allowed_access) + return -EACCES; + + /* + * For operations on already opened files (i.e. ftruncate()), it is the + * access rights at the time of open() which decide whether the + * operation is permitted. Therefore, we record the relevant subset of + * file access rights in the opened struct file. + */ + landlock_file(file)->allowed_access = allowed_access; + return 0; +} + +static int hook_file_truncate(struct file *const file) +{ + /* + * Allows truncation if the truncate right was available at the time of + * opening the file, to get a consistent access check as for read, write + * and execute operations. + * + * Note: For checks done based on the file's Landlock rights, we enforce + * them independently of whether the current thread is in a Landlock + * domain, so that open files passed between independent processes + * retain their behaviour. + */ + if (landlock_file(file)->allowed_access & LANDLOCK_ACCESS_FS_TRUNCATE) + return 0; + return -EACCES; } static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { @@ -1193,6 +1281,8 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = { LSM_HOOK_INIT(path_symlink, hook_path_symlink), LSM_HOOK_INIT(path_unlink, hook_path_unlink), LSM_HOOK_INIT(path_rmdir, hook_path_rmdir), + LSM_HOOK_INIT(path_truncate, hook_path_truncate), + LSM_HOOK_INIT(file_truncate, hook_file_truncate), LSM_HOOK_INIT(file_open, hook_file_open), }; diff --git a/security/landlock/fs.h b/security/landlock/fs.h index 8db7acf9109b..488e4813680a 100644 --- a/security/landlock/fs.h +++ b/security/landlock/fs.h @@ -36,6 +36,24 @@ struct landlock_inode_security { struct landlock_object __rcu *object; }; +/** + * struct landlock_file_security - File security blob + * + * This information is populated when opening a file in hook_file_open, and + * tracks the relevant Landlock access rights that were available at the time + * of opening the file. Other LSM hooks use these rights in order to authorize + * operations on already opened files. + */ +struct landlock_file_security { + /** + * @allowed_access: Access rights that were available at the time of + * opening the file. This is not necessarily the full set of access + * rights available at that time, but it's the necessary subset as + * needed to authorize later operations on the open file. + */ + access_mask_t allowed_access; +}; + /** * struct landlock_superblock_security - Superblock security blob * @@ -50,6 +68,12 @@ struct landlock_superblock_security { atomic_long_t inode_refs; }; +static inline struct landlock_file_security * +landlock_file(const struct file *const file) +{ + return file->f_security + landlock_blob_sizes.lbs_file; +} + static inline struct landlock_inode_security * landlock_inode(const struct inode *const inode) { diff --git a/security/landlock/limits.h b/security/landlock/limits.h index b54184ab9439..82288f0e9e5e 100644 --- a/security/landlock/limits.h +++ b/security/landlock/limits.h @@ -18,7 +18,7 @@ #define LANDLOCK_MAX_NUM_LAYERS 16 #define LANDLOCK_MAX_NUM_RULES U32_MAX -#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_REFER +#define LANDLOCK_LAST_ACCESS_FS LANDLOCK_ACCESS_FS_TRUNCATE #define LANDLOCK_MASK_ACCESS_FS ((LANDLOCK_LAST_ACCESS_FS << 1) - 1) #define LANDLOCK_NUM_ACCESS_FS __const_hweight64(LANDLOCK_MASK_ACCESS_FS) diff --git a/security/landlock/setup.c b/security/landlock/setup.c index f8e8e980454c..3f196d2ce4f9 100644 --- a/security/landlock/setup.c +++ b/security/landlock/setup.c @@ -19,6 +19,7 @@ bool landlock_initialized __lsm_ro_after_init = false; struct lsm_blob_sizes landlock_blob_sizes __lsm_ro_after_init = { .lbs_cred = sizeof(struct landlock_cred_security), + .lbs_file = sizeof(struct landlock_file_security), .lbs_inode = sizeof(struct landlock_inode_security), .lbs_superblock = sizeof(struct landlock_superblock_security), }; diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index 735a0865ea11..f4d6fc7ed17f 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -129,7 +129,7 @@ static const struct file_operations ruleset_fops = { .write = fop_dummy_write, }; -#define LANDLOCK_ABI_VERSION 2 +#define LANDLOCK_ABI_VERSION 3 /** * sys_landlock_create_ruleset - Create a new ruleset diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index da9290817866..72cdae277b02 100644 --- a/tools/testing/selftests/landlock/base_test.c +++ b/tools/testing/selftests/landlock/base_test.c @@ -75,7 +75,7 @@ TEST(abi_version) const struct landlock_ruleset_attr ruleset_attr = { .handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE, }; - ASSERT_EQ(2, landlock_create_ruleset(NULL, 0, + ASSERT_EQ(3, landlock_create_ruleset(NULL, 0, LANDLOCK_CREATE_RULESET_VERSION)); ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0, diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 45de42a027c5..87b28d14a1aa 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -406,9 +406,10 @@ TEST_F_FORK(layout1, inval) #define ACCESS_FILE ( \ LANDLOCK_ACCESS_FS_EXECUTE | \ LANDLOCK_ACCESS_FS_WRITE_FILE | \ - LANDLOCK_ACCESS_FS_READ_FILE) + LANDLOCK_ACCESS_FS_READ_FILE | \ + LANDLOCK_ACCESS_FS_TRUNCATE) -#define ACCESS_LAST LANDLOCK_ACCESS_FS_REFER +#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE #define ACCESS_ALL ( \ ACCESS_FILE | \ @@ -422,7 +423,7 @@ TEST_F_FORK(layout1, inval) LANDLOCK_ACCESS_FS_MAKE_FIFO | \ LANDLOCK_ACCESS_FS_MAKE_BLOCK | \ LANDLOCK_ACCESS_FS_MAKE_SYM | \ - ACCESS_LAST) + LANDLOCK_ACCESS_FS_REFER) /* clang-format on */
Introduce the LANDLOCK_ACCESS_FS_TRUNCATE flag for file truncation. This flag hooks into the path_truncate LSM hook and covers file truncation using truncate(2), ftruncate(2), open(2) with O_TRUNC, as well as creat(). This change also increments the Landlock ABI version, updates corresponding selftests, and updates code documentation to document the flag. The following operations are restricted: open(): requires the LANDLOCK_ACCESS_FS_TRUNCATE right if a file gets implicitly truncated as part of the open() (e.g. using O_TRUNC). Notable special cases: * open(..., O_RDONLY|O_TRUNC) can truncate files as well in Linux * open() with O_TRUNC does *not* need the TRUNCATE right when it creates a new file. truncate() (on a path): requires the LANDLOCK_ACCESS_FS_TRUNCATE right. ftruncate() (on a file): requires that the file had the TRUNCATE right when it was previously opened. Signed-off-by: Günther Noack <gnoack3000@gmail.com> --- include/uapi/linux/landlock.h | 21 +++- security/landlock/fs.c | 102 +++++++++++++++++-- security/landlock/fs.h | 24 +++++ security/landlock/limits.h | 2 +- security/landlock/setup.c | 1 + security/landlock/syscalls.c | 2 +- tools/testing/selftests/landlock/base_test.c | 2 +- tools/testing/selftests/landlock/fs_test.c | 7 +- 8 files changed, 144 insertions(+), 17 deletions(-)