| Message ID | 855c5c7d-586a-3b35-76f8-ddca4fae1410@nasa.gov (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 12, 2017 at 02:40:11PM -0700, Jay Lan wrote: > Hi all, > > I was writing a livepatch patch, but compilation failed in creating the .ko > because mount_lock is "undefined." > > The mount_lock is defined globally in fs/namespace.c > __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); > > and extern in fs/mount.h. > extern seqlock_t mount_lock; > > If there is a reason that mount_lock should not be EXPORT_SYMBOL_GPL, please > advise; otherwise, I propose the patch to export this symbol. Because there is no %$^$^ reason to use it from a module ever?
On 07/13/2017 12:15 AM, Christoph Hellwig wrote: > On Wed, Jul 12, 2017 at 02:40:11PM -0700, Jay Lan wrote: >> Hi all, >> >> I was writing a livepatch patch, but compilation failed in creating the .ko >> because mount_lock is "undefined." >> >> The mount_lock is defined globally in fs/namespace.c >> __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); >> >> and extern in fs/mount.h. >> extern seqlock_t mount_lock; >> >> If there is a reason that mount_lock should not be EXPORT_SYMBOL_GPL, please >> advise; otherwise, I propose the patch to export this symbol. > Because there is no %$^$^ reason to use it from a module ever? Is livepatch a good reason? Besides security fixes, livepatch is a good tool to use for debugging and providing a temporary fix on production systems (until next release from vendors) I know this argument is not strong, but at least mount_lock is declared globally. Yes? Thanks, Jay
On Thu, Jul 13, 2017 at 11:24:01AM -0700, Jay Lan wrote: > On 07/13/2017 12:15 AM, Christoph Hellwig wrote: > > On Wed, Jul 12, 2017 at 02:40:11PM -0700, Jay Lan wrote: > > > Hi all, > > > > > > I was writing a livepatch patch, but compilation failed in creating the .ko > > > because mount_lock is "undefined." > > > > > > The mount_lock is defined globally in fs/namespace.c > > > __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); > > > > > > and extern in fs/mount.h. > > > extern seqlock_t mount_lock; > > > > > > If there is a reason that mount_lock should not be EXPORT_SYMBOL_GPL, please > > > advise; otherwise, I propose the patch to export this symbol. > > Because there is no %$^$^ reason to use it from a module ever? > Is livepatch a good reason? Besides security fixes, livepatch is a good tool > to use for debugging and providing a temporary fix on production systems > (until next release from vendors) I know this argument is not strong, but at > least mount_lock is declared globally. Yes? Something I've done in the past as a local hack when a symbol I wanted wasn't exported is: extern seqlock_t *mount_lock_p; module_param(mount_lock_p, charp, 0); and then pass the address (from /proc/kallsyms) as a module parameter at load time.
On 07/13/2017 11:58 AM, Matthew Wilcox wrote: > On Thu, Jul 13, 2017 at 11:24:01AM -0700, Jay Lan wrote: >> On 07/13/2017 12:15 AM, Christoph Hellwig wrote: >>> On Wed, Jul 12, 2017 at 02:40:11PM -0700, Jay Lan wrote: >>>> Hi all, >>>> >>>> I was writing a livepatch patch, but compilation failed in creating the .ko >>>> because mount_lock is "undefined." >>>> >>>> The mount_lock is defined globally in fs/namespace.c >>>> __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); >>>> >>>> and extern in fs/mount.h. >>>> extern seqlock_t mount_lock; >>>> >>>> If there is a reason that mount_lock should not be EXPORT_SYMBOL_GPL, please >>>> advise; otherwise, I propose the patch to export this symbol. >>> Because there is no %$^$^ reason to use it from a module ever? >> Is livepatch a good reason? Besides security fixes, livepatch is a good tool >> to use for debugging and providing a temporary fix on production systems >> (until next release from vendors) I know this argument is not strong, but at >> least mount_lock is declared globally. Yes? > Something I've done in the past as a local hack when a symbol I wanted > wasn't exported is: > > extern seqlock_t *mount_lock_p; > module_param(mount_lock_p, charp, 0); > > and then pass the address (from /proc/kallsyms) as a module parameter > at load time. Thanks for the tip, Matthew! Please considered the patch request withdrawn. Jay
On Thu, Jul 13, 2017 at 11:24:01AM -0700, Jay Lan wrote: > On 07/13/2017 12:15 AM, Christoph Hellwig wrote: > > On Wed, Jul 12, 2017 at 02:40:11PM -0700, Jay Lan wrote: > > > Hi all, > > > > > > I was writing a livepatch patch, but compilation failed in creating the .ko > > > because mount_lock is "undefined." > > > > > > The mount_lock is defined globally in fs/namespace.c > > > __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); > > > > > > and extern in fs/mount.h. > > > extern seqlock_t mount_lock; > > > > > > If there is a reason that mount_lock should not be EXPORT_SYMBOL_GPL, please > > > advise; otherwise, I propose the patch to export this symbol. > > Because there is no %$^$^ reason to use it from a module ever? > Is livepatch a good reason? Besides security fixes, livepatch is a good tool > to use for debugging and providing a temporary fix on production systems > (until next release from vendors) I know this argument is not strong, but at > least mount_lock is declared globally. Yes? No. If you are playing with mount_lock, the odds are that you are accessing the data structures declared in fs/mount.h. They are not in include/* for a damn good reason - we want to be able to change them at zero notice in any way we bloody well like. Without having to locate someone's code that stuck its private parts into the machinery. "Not static in file" != "everyone is welcome to play with that area". Again, *ANY* module playing with the stuff in struct mount, let alone the hashes, etc. is in position to play Charlie Chaplin's Modern Times stunt - the one with travel through the gears. At zero notice. With no apologies for the pain experienced by bereft users or for unkind replies to resulting complaints. You Have Been Warned: DON'T GO THERE. I don't know if I can make it any more clear short of running afoul of CDA and its ilk.
diff --git a/fs/namespace.c b/fs/namespace.c index 5a4438445bf7..3bbb619391ce 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -83,6 +83,7 @@ EXPORT_SYMBOL_GPL(fs_kobj); * tree or hash is modified or when a vfsmount structure is modified. */ __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); +EXPORT_SYMBOL_GPL(mount_lock); static inline struct hlist_head *m_hash(struct vfsmount *mnt, struct dentry *dentry)
Hi all, I was writing a livepatch patch, but compilation failed in creating the .ko because mount_lock is "undefined." The mount_lock is defined globally in fs/namespace.c __cacheline_aligned_in_smp DEFINE_SEQLOCK(mount_lock); and extern in fs/mount.h. extern seqlock_t mount_lock; If there is a reason that mount_lock should not be EXPORT_SYMBOL_GPL, please advise; otherwise, I propose the patch to export this symbol. Signed-off-by: Jay Lan <jay.j.lan@nasa.gov> --- fs/namespace.c | 1 + 1 file changed, 1 insertion(+) {