- 30 Aug, 2024 30 commits
-
-
Julian Sun authored
Hi, all Recently I noticed a bug[1] in btrfs, after digged it into and I believe it'a race in vfs. Let's assume there's a inode (ie ino 261) with i_count 1 is called by iput(), and there's a concurrent thread calling generic_shutdown_super(). cpu0: cpu1: iput() // i_count is 1 ->spin_lock(inode) ->dec i_count to 0 ->iput_final() generic_shutdown_super() ->__inode_add_lru() ->evict_inodes() // cause some reason[2] ->if (atomic_read(inode->i_count)) continue; // return before // inode 261 passed the above check // list_lru_add_obj() // and then schedule out ->spin_unlock() // note here: the inode 261 // was still at sb list and hash list, // and I_FREEING|I_WILL_FREE was not been set btrfs_iget() // after some function calls ->find_inode() // found the above inode 261 ->spin_lock(inode) // check I_FREEING|I_WILL_FREE // and passed ->__iget() ->spin_unlock(inode) // schedule back ->spin_lock(inode) // check (I_NEW|I_FREEING|I_WILL_FREE) flags, // passed and set I_FREEING iput() ->spin_unlock(inode) ->spin_lock(inode) ->evict() // dec i_count to 0 ->iput_final() ->spin_unlock() ->evict() Now, we have two threads simultaneously evicting the same inode, which may trigger the BUG(inode->i_state & I_CLEAR) statement both within clear_inode() and iput(). To fix the bug, recheck the inode->i_count after holding i_lock. Because in the most scenarios, the first check is valid, and the overhead of spin_lock() can be reduced. If there is any misunderstanding, please let me know, thanks. [1]: https://lore.kernel.org/linux-btrfs/000000000000eabe1d0619c48986@google.com/ [2]: The reason might be 1. SB_ACTIVE was removed or 2. mapping_shrinkable() return false when I reproduced the bug. Reported-by: syzbot+67ba3c42bcbb4665d3ad@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=67ba3c42bcbb4665d3ad CC: stable@vger.kernel.org Fixes: 63997e98 ("split invalidate_inodes()") Signed-off-by:Julian Sun <sunjunchao2870@gmail.com> Link: https://lore.kernel.org/r/20240823130730.658881-1-sunjunchao2870@gmail.comReviewed-by:
Jan Kara <jack@suse.cz> Signed-off-by:
Christian Brauner <brauner@kernel.org>
-
Eric Biggers authored
The VFS git tree is missing from MAINTAINERS. Add it. Signed-off-by:
Eric Biggers <ebiggers@google.com> Link: https://lore.kernel.org/r/20240820195109.38906-1-ebiggers@kernel.orgSigned-off-by:
-
Christian Brauner authored
The underscore variants are for uapi whereas the non-underscore variants are for in-kernel consumers. Link: https://lore.kernel.org/r/20240822-anwerben-nutzung-1cd6c82a565f@braunerReviewed-by:
-
Christian Brauner authored
This helper has been unused for a while now. Link: https://lore.kernel.org/r/20240822-bewuchs-werktag-46672b3c0606@braunerReviewed-by:
-
Mateusz Guzik authored
Most commonly neither I_LRU_ISOLATING nor I_SYNC are set, but the stock kernel takes a back-to-back relock trip to check for them. It probably can be avoided altogether, but for now massage things back to just one lock acquire. Signed-off-by:
Mateusz Guzik <mjguzik@gmail.com> Link: https://lore.kernel.org/r/20240813143626.1573445-1-mjguzik@gmail.comReviewed-by:
Zhihao Cheng <chengzhihao1@huawei.com> Reviewed-by:
Jeff Layton <jlayton@kernel.org> Reviewed-by:
-
Marc Aurèle La France authored
After its conversion to the new mount API, debugfs displays "none" in /proc/mounts instead of the actual source. Fix this by recognising its "source" mount option. Signed-off-by:
Marc Aurèle La France <tsi@tuyoix.net> Link: https://lore.kernel.org/r/e439fae2-01da-234b-75b9-2a7951671e27@tuyoix.net Fixes: a20971c1 ("vfs: Convert debugfs to use the new mount API") Cc: stable@vger.kernel.org # 6.10.x: 49abee59: debugfs: Convert to new uid/gid option parsing helpers Signed-off-by:
-
Christian Brauner authored
Afaict, we can just rely on inode->i_dio_count for waiting instead of this awkward indirection through __I_DIO_WAKEUP. This survives LTP dio and xfstests dio tests. Link: https://lore.kernel.org/r/20240816-vfs-misc-dio-v1-1-80fe21a2c710@kernel.orgReviewed-by:
Josef Bacik <josef@toxicpanda.com> Reviewed-by:
-
Hongbo Li authored
In step 2, we obtain the kernel id `k1000`. So in next step (step 3), we should translate the `k1000` not `k21000`. Signed-off-by:
Hongbo Li <lihongbo22@huawei.com> Link: https://lore.kernel.org/r/20240816063611.1961910-1-lihongbo22@huawei.comSigned-off-by:
-
Hongbo Li authored
Since in_group_or_capable has been exported, we can use it to simplify the code when check group and capable. Signed-off-by:
-
Mateusz Guzik authored
According to bpftrace on these routines most calls result in cmpxchg, which already provides the same guarantee. In inode_maybe_inc_iversion elision is possible because even if the wrong value was read due to now missing smp_mb fence, the issue is going to correct itself after cmpxchg. If it appears cmpxchg wont be issued, the fence + reload are there bringing back previous behavior. Signed-off-by:
-
Ian Kent authored
Add ability to set per-dentry mount expire timeout to autofs. There are two fairly well known automounter map formats, the autofs format and the amd format (more or less System V and Berkley). Some time ago Linux autofs added an amd map format parser that implemented a fair amount of the amd functionality. This was done within the autofs infrastructure and some functionality wasn't implemented because it either didn't make sense or required extra kernel changes. The idea was to restrict changes to be within the existing autofs functionality as much as possible and leave changes with a wider scope to be considered later. One of these changes is implementing the amd options: 1) "unmount", expire this mount according to a timeout (same as the current autofs default). 2) "nounmount", don't expire this mount (same as setting the autofs timeout to 0 except only for this specific mount) . 3) "utimeout=<seconds>", expire this mount using the specified timeout (again same as setting the autofs timeout but only for this mount). To implement these options per-dentry expire timeouts need to be implemented for autofs indirect mounts. This is because all map keys (mounts) for autofs indirect mounts use an expire timeout stored in the autofs mount super block info. structure and all indirect mounts use the same expire timeout. Now I have a request to add the "nounmount" option so I need to add the per-dentry expire handling to the kernel implementation to do this. The implementation uses the trailing path component to identify the mount (and is also used as the autofs map key) which is passed in the autofs_dev_ioctl structure path field. The expire timeout is passed in autofs_dev_ioctl timeout field (well, of the timeout union). If the passed in timeout is equal to -1 the per-dentry timeout and flag are cleared providing for the "unmount" option. If the timeout is greater than or equal to 0 the timeout is set to the value and the flag is also set. If the dentry timeout is 0 the dentry will not expire by timeout which enables the implementation of the "nounmount" option for the specific mount. When the dentry timeout is greater than zero it allows for the implementation of the "utimeout=<seconds>" option. Signed-off-by:
Ian Kent <raven@themaw.net> Link: https://lore.kernel.org/r/20240814090231.963520-1-raven@themaw.netSigned-off-by:
-
Mateusz Guzik authored
A soft lockup in ilookup was reported when stress-testing a 512-way system [1] (see [2] for full context) and it was verified that not taking the lock shifts issues back to mm. [1] https://lore.kernel.org/linux-mm/56865e57-c250-44da-9713-cf1404595bcc@amd.com/ [2] https://lore.kernel.org/linux-mm/d2841226-e27b-4d3d-a578-63587a3aa4f3@amd.com/Signed-off-by:
-
Christian Brauner authored
This is another flag that is statically set and doesn't need to use up an FMODE_* bit. Move it to ->fop_flags and free up another FMODE_* bit. (1) mem_open() used from proc_mem_operations (2) adi_open() used from adi_fops (3) drm_open_helper(): (3.1) accel_open() used from DRM_ACCEL_FOPS (3.2) drm_open() used from (3.2.1) amdgpu_driver_kms_fops (3.2.2) psb_gem_fops (3.2.3) i915_driver_fops (3.2.4) nouveau_driver_fops (3.2.5) panthor_drm_driver_fops (3.2.6) radeon_driver_kms_fops (3.2.7) tegra_drm_fops (3.2.8) vmwgfx_driver_fops (3.2.9) xe_driver_fops (3.2.10) DRM_GEM_FOPS (3.2.11) DEFINE_DRM_GEM_DMA_FOPS (4) struct memdev sets fmode flags based on type of device opened. For devices using struct mem_fops unsigned offset is used. Mark all these file operations as FOP_UNSIGNED_OFFSET and add asserts into the open helper to ensure that the flag is always set. Link: https://lore.kernel.org/r/20240809-work-fop_unsigned-v1-1-658e054d893e@kernel.orgReviewed-by: -
Mateusz Guzik authored
In do_dentry_open() the usage is: f->f_op = fops_get(inode->i_fop); In generated asm the compiler emits 2 reads from inode->i_fop instead of just one. This popped up due to false-sharing where loads from that offset end up bouncing a cacheline during parallel open. While this is going to be fixed, the spurious load does not need to be there. This makes do_dentry_open() go down from 1177 to 1154 bytes. fops_put() is patched to maintain some consistency. No functional changes. Reviewed-by:
-
Thorsten Blum authored
Add the __counted_by compiler attribute to the flexible array member entries to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE. Signed-off-by:
Thorsten Blum <thorsten.blum@toblux.com> Link: https://lore.kernel.org/r/20240808150023.72578-2-thorsten.blum@toblux.comReviewed-by:
Gustavo A. R. Silva <gustavoars@kernel.org> Signed-off-by:
-
Christian Brauner authored
If we find a positive dentry we can now simply try and open it. All prelimiary checks are already done with or without O_CREAT. Signed-off-by: -
Christian Brauner authored
Now that we audit later during lookup_open() we can remove the audit dummy context check. This simplifies things a lot. Signed-off-by: -
Christian Brauner authored
Perform the check for trailing slashes right in the fastpath check and don't bother with any additional work. Signed-off-by: -
Christian Brauner authored
During O_CREAT we unconditionally audit the parent inode. This makes it difficult to support a fastpath for O_CREAT when the file already exists because we have to drop out of RCU lookup needlessly. We worked around this by checking whether audit was actually active but that's also suboptimal. Instead, move the audit of the parent inode down into lookup_open() at a point where it's mostly certain that the file needs to be created. This also reduced the inconsistency that currently exists: while audit on the parent is done independent of whether or no the file already existed an audit on the file is only performed if it has been created. By moving the audit down a bit we emit the audit a little later but it will allow us to simplify the fastpath for O_CREAT significantly. Signed-off-by: -
Jeff Layton authored
Today, when opening a file we'll typically do a fast lookup, but if O_CREAT is set, the kernel always takes the exclusive inode lock. I assume this was done with the expectation that O_CREAT means that we always expect to do the create, but that's often not the case. Many programs set O_CREAT even in scenarios where the file already exists. This patch rearranges the pathwalk-for-open code to also attempt a fast_lookup in certain O_CREAT cases. If a positive dentry is found, the inode_lock can be avoided altogether, and if auditing isn't enabled, it can stay in rcuwalk mode for the last step_into. One notable exception that is hopefully temporary: if we're doing an rcuwalk and auditing is enabled, skip the lookup_fast. Legitimizing the dentry in that case is more expensive than taking the i_rwsem for now. Signed-off-by:
-
Martin Karsten authored
A struct eventpoll's busy_poll_usecs field can be modified via a user ioctl at any time. All reads of this field should be annotated with READ_ONCE. Fixes: 85455c79 ("eventpoll: support busy poll per epoll instance") Cc: stable@vger.kernel.org Signed-off-by:
Martin Karsten <mkarsten@uwaterloo.ca> Link: https://lore.kernel.org/r/20240806123301.167557-1-jdamato@fastly.comReviewed-by:
Joe Damato <jdamato@fastly.com> Signed-off-by:
-
Joe Damato authored
Remove redundant and unnecessary code. ep_alloc uses kzalloc to create struct eventpoll, so there is no need to set fields to defaults of 0. This was accidentally introduced in commit 85455c79 ("eventpoll: support busy poll per epoll instance") and expanded on in follow-up commits. Signed-off-by:
-
Xiaxi Shen authored
Fixed 3 typos in design.rst Signed-off-by:
Xiaxi Shen <shenxiaxi26@gmail.com> Link: https://lore.kernel.org/r/20240807070536.14536-1-shenxiaxi26@gmail.comReviewed-by:
Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by:
Darrick J. Wong <djwong@kernel.org> Signed-off-by:
-
Mateusz Guzik authored
These inlines show up in the fast path (e.g., in do_dentry_open()) and induce said full barrier regarding i_flctx access when in most cases the pointer is NULL. The pointer can be safely checked before issuing the barrier, dodging it in most cases as a result. It is plausible the consume fence would be sufficient, but I don't want to go audit all callers regarding what they before calling here. Signed-off-by:
-
Joel Savitz authored
Cc: Alexander Viro <viro@zeniv.linux.org.uk> Cc: Christian Brauner <brauner@kernel.org> Cc: Jan Kara <jack@suse.cz> Cc: linux-fsdevel@vger.kernel.org The comment on EXPORT_SYMBOL(close_fd) was added in commit 2ca2a09d ("fs: add ksys_close() wrapper; remove in-kernel calls to sys_close()"), before commit 8760c909 ("file: Rename __close_fd to close_fd and remove the files parameter") gave the function its current name, however commit 1572bfdf ("file: Replace ksys_close with close_fd") removes the referenced caller entirely, obsoleting this comment. Signed-off-by:
Joel Savitz <jsavitz@redhat.com> Link: https://lore.kernel.org/r/20240803025455.239276-1-jsavitz@redhat.comReviewed-by:
-
Yuesong Li authored
replace 'permanetly' with 'permanently' in the comment & replace 'propogated' with 'propagated' in the comment Signed-off-by:
Yuesong Li <liyuesong@vivo.com> Link: https://lore.kernel.org/r/20240806034710.2807788-1-liyuesong@vivo.comSigned-off-by:
-
Mateusz Guzik authored
Both i_mode and noexec checks wrapped in WARN_ON stem from an artifact of the previous implementation. They used to legitimately check for the condition, but that got moved up in two commits: 633fb6ac ("exec: move S_ISREG() check earlier") 0fd338b2 ("exec: move path_noexec() check earlier") Instead of being removed said checks are WARN_ON'ed instead, which has some debug value. However, the spurious path_noexec check is racy, resulting in unwarranted warnings should someone race with setting the noexec flag. One can note there is more to perm-checking whether execve is allowed and none of the conditions are guaranteed to still hold after they were tested for. Additionally this does not validate whether the code path did any perm checking to begin with -- it will pass if the inode happens to be regular. Keep the redundant path_noexec() check even though it's mindless nonsense checking for guarantee that isn't given so drop the WARN. Reword the commentary and do small tidy ups while here. Signed-off-by:
-
Jeff Layton authored
The lookup_fast helper in fs/namei.c has some subtlety in how dentries are returned. Document them. Signed-off-by:
-
Jeff Layton authored
This function no longer exists. Signed-off-by:
-
Yue Haibing authored
Commit 2eea9ce4 ("mounts: keep list of mounts in an rbtree") removed the implementation but leave declaration. Signed-off-by:
Yue Haibing <yuehaibing@huawei.com> Link: https://lore.kernel.org/r/20240803115000.589872-1-yuehaibing@huawei.comSigned-off-by:
-
- 19 Aug, 2024 10 commits
-
-
Wang Long authored
In the function percpu_rwsem_release, the parameter `read` is unused, so remove it. Signed-off-by:
Wang Long <w@laoqinren.net> Link: https://lore.kernel.org/r/20240802091901.2546797-1-w@laoqinren.netReviewed-by:
-
Aleksa Sarai authored
While the old code did support FSCONFIG_SET_FD, there's no need to re-get the file the fs_context infrastructure already grabbed for us. Signed-off-by:
Aleksa Sarai <cyphar@cyphar.com> Link: https://lore.kernel.org/r/20240731-fsconfig-fsparam_fd-fixes-v2-2-e7c472224417@cyphar.comSigned-off-by:
-
Aleksa Sarai authored
If you pass an fd using FSCONFIG_SET_FD, autofs_parse_fd() "steals" the param->file and so the fs_context infrastructure will not do fput() for us. Fixes: e6ec453b ("autofs: convert autofs to use the new mount api") Signed-off-by:
-
Uros Bizjak authored
__percpu annotation of *cpu pointer in struct kioctx is put at the wrong place, resulting in several sparse warnings: aio.c:623:24: warning: incorrect type in argument 1 (different address spaces) aio.c:623:24: expected void [noderef] __percpu *__pdata aio.c:623:24: got struct kioctx_cpu *cpu aio.c:788:18: warning: incorrect type in assignment (different address spaces) aio.c:788:18: expected struct kioctx_cpu *cpu aio.c:788:18: got struct kioctx_cpu [noderef] __percpu * aio.c:835:24: warning: incorrect type in argument 1 (different address spaces) aio.c:835:24: expected void [noderef] __percpu *__pdata aio.c:835:24: got struct kioctx_cpu *cpu aio.c:940:16: warning: incorrect type in initializer (different address spaces) aio.c:940:16: expected void const [noderef] __percpu *__vpp_verify aio.c:940:16: got struct kioctx_cpu * aio.c:958:16: warning: incorrect type in initializer (different address spaces) aio.c:958:16: expected void const [noderef] __percpu *__vpp_verify aio.c:958:16: got struct kioctx_cpu * Put __percpu annotation at the right place to fix these warnings. Signed-off-by:
Uros Bizjak <ubizjak@gmail.com> Link: https://lore.kernel.org/r/20240730121915.4514-1-ubizjak@gmail.comReviewed-by:
-
Olaf Hering authored
If no page could be allocated, an error pointer was used as format string in pr_warn. Rearrange the code to return early in case of OOM. Also add a check for the return value of d_path. Fixes: f8b92ba6 ("mount: Add mount warning for impending timestamp expiry") Signed-off-by:
Olaf Hering <olaf@aepfle.de> Link: https://lore.kernel.org/r/20240730085856.32385-1-olaf@aepfle.de [brauner: rewrite commit and commit message] Signed-off-by:
-
Mohit0404 authored
Fixed- WARNING: Missing a blank line after declarations WARNING: Missing a blank line after declarations Declaration format: improved struct file declaration format Signed-off-by:
Mohit0404 <mohitpawar@mitaoe.ac.in> Link: https://lore.kernel.org/r/20240727072134.130962-2-mohitpawar@mitaoe.ac.inReviewed-by:
-
Youling Tang authored
After commit c22198e7 ("direct-io: remove random prefetches"), Nothing in this file needs anything from `linux/prefetch.h`. Signed-off-by:
Youling Tang <tangyouling@kylinos.cn> Link: https://lore.kernel.org/r/20240603014834.45294-1-youling.tang@linux.devReviewed-by:
-
Haifeng Xu authored
When deactivating any type of superblock, it had to wait for the in-flight wb switches to be completed. wb switches are executed in inode_switch_wbs_work_fn() which needs to acquire the wb_switch_rwsem and races against sync_inodes_sb(). If there are too much dirty data in the superblock, the waiting time may increase significantly. For superblocks without cgroup writeback such as tmpfs, they have nothing to do with the wb swithes, so the flushing can be avoided. Signed-off-by:
Haifeng Xu <haifeng.xu@shopee.com> Link: https://lore.kernel.org/r/20240726030525.180330-1-haifeng.xu@shopee.comReviewed-by:
-
Christian Brauner authored
Christian Brauner <brauner@kernel.org> says: Systemd has a helper called openat_report_new() that returns whether a file was created anew or it already existed before for cases where O_CREAT has to be used without O_EXCL (cf. [1]). That apparently isn't something that's specific to systemd but it's where I noticed it. The current logic is that it first attempts to open the file without O_CREAT | O_EXCL and if it gets ENOENT the helper tries again with both flags. If that succeeds all is well. If it now reports EEXIST it retries. That works fairly well but some corner cases make this more involved. If this operates on a dangling symlink the first openat() without O_CREAT | O_EXCL will return ENOENT but the second openat() with O_CREAT | O_EXCL will fail with EEXIST. The reason is that openat() without O_CREAT | O_EXCL follows the symlink while O_CREAT | O_EXCL doesn't for security reasons. So it's not something we can really change unless we add an explicit opt-in via O_FOLLOW which seems really ugly. The caller could try and use fanotify() to register to listen for creation events in the directory before calling openat(). The caller could then compare the returned tid to its own tid to ensure that even in threaded environments it actually created the file. That might work but is a lot of work for something that should be fairly simple and I'm uncertain about it's reliability. The caller could use a bpf lsm hook to hook into security_file_open() to figure out whether they created the file. That also seems a bit wild. So let's add F_CREATED_QUERY which allows the caller to check whether they actually did create the file. That has caveats of course but I don't think they are problematic: * In multi-threaded environments a thread can only be sure that it did create the file if it calls openat() with O_CREAT. In other words, it's obviously not enough to just go through it's fdtable and check these fds because another thread could've created the file. * If there's any codepaths where an openat() with O_CREAT would yield the same struct file as that of another thread it would obviously cause wrong results. I'm not aware of any such codepaths from openat() itself. Imho, that would be a bug. * Related to the previous point, calling the new fcntl() on files created and opened via special-purpose system calls or ioctl()s would cause wrong results only if the affected subsystem a) raises FMODE_CREATED and b) may return the same struct file for two different calls. I'm not seeing anything outside of regular VFS code that raises FMODE_CREATED. There is code for b) in e.g., the drm layer where the same struct file is resurfaced but again FMODE_CREATED isn't used and it would be very misleading if it did. [1]: https://github.com/systemd/systemd/blob/11d5e2b5fbf9f6bfa5763fd45b56829ad4f0777f/src/basic/fs-util.c#L1078 * patches from https://lore.kernel.org/r/20240724-work-fcntl-v1-0-e8153a2f1991@kernel.org: selftests: add F_CREATED_QUERY tests fcntl: add F_CREATED_QUERY Signed-off-by:
-
Christian Brauner authored
Add simple selftests for fcntl(fd, F_CREATED_QUERY, 0). Link: https://lore.kernel.org/r/20240724-work-fcntl-v1-2-e8153a2f1991@kernel.orgSigned-off-by:
-
