[RFC PATCH 2/2] jbd2: avoid clobbering registers with J_ASSERT macro
From: Chris Snook <[EMAIL PROTECTED]>
Don't printk before BUG in J_ASSERT unless CONFIG_JBD2_DEBUG is set.
Signed-off-by: Chris Snook <[EMAIL PROTECTED]>
--- linux-2.6.23-rc3-orig/include/linux/jbd2.h 2007-08-13 03:14:13.0
-0400
+++ linux-2.6.23-rc3-patch/include/linux/jbd2.h 2007-08-17 01:44:34.0
-0400
@@ -255,7 +255,10 @@ typedef struct journal_superblock_s
#include
#include
+#ifdef CONFIG_JBD2_DEBUG
#define JBD_ASSERTIONS
+#endif
+
#ifdef JBD_ASSERTIONS
#define J_ASSERT(assert) \
do { \
@@ -282,7 +285,7 @@ void buffer_assertion_failure(struct buf
#endif
#else
-#define J_ASSERT(assert) do { } while (0)
+#define J_ASSERT(assert) BUG_ON(!(assert))
#endif /* JBD_ASSERTIONS */
#if defined(JBD_PARANOID_IOFAIL)
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/2] jbd: avoid clobbering registers with J_ASSERT macro
From: Chris Snook <[EMAIL PROTECTED]>
Don't printk before BUG in J_ASSERT unless CONFIG_JBD_DEBUG is set.
Signed-off-by: Chris Snook <[EMAIL PROTECTED]>
--- linux-2.6.23-rc3-orig/include/linux/jbd.h 2007-07-08 19:32:17.0
-0400
+++ linux-2.6.23-rc3-patch/include/linux/jbd.h 2007-08-17 01:43:34.0
-0400
@@ -246,7 +246,10 @@ typedef struct journal_superblock_s
#include
#include
+#ifdef CONFIG_JBD_DEBUG
#define JBD_ASSERTIONS
+#endif
+
#ifdef JBD_ASSERTIONS
#define J_ASSERT(assert) \
do { \
@@ -273,7 +276,7 @@ void buffer_assertion_failure(struct buf
#endif
#else
-#define J_ASSERT(assert) do { } while (0)
+#define J_ASSERT(assert) BUG_ON(!(assert))
#endif /* JBD_ASSERTIONS */
#if defined(JBD_PARANOID_IOFAIL)
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/2] avoid clobbering registers with J_ASSERT macro
The J_ASSERT() macro in jbd and jbd2 calls printk() prior to BUG(). While this makes it more convenient to read the assertion failure, it also clobbers registers, which can sometimes make debugging harder, which is clearly not the intended purpose. I recently banged my head on this myself. The following patches to jbd and jbd2 enable the printk only if CONFIG_JBD[2]_DEBUG is set. Otherwise, it will simply BUG if the condition is violated. This way test kernels still get the benefit of the J_ASSERT printk, while production kernels, which come from a more stable source base where it's easier to trace line numbers back to specific lines of code, simply get the BUG, with all registers preserved. This is, of course, not the only way of fixing this problem, but it seems to be the least invasive way, which is why I'm proposing these patches. -- Chris - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Aug 16, 2007, at 18:57:24, Linus Torvalds wrote: On Wed, 15 Aug 2007, David Howells wrote: Would you object greatly to functions like vfs_mkdir() gaining a security parameter? What I'm thinking of is this: int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode, struct security *security) I personally consider this an affront to everythign that is decent. Why the *hell* would mkdir() be so magical as to need something like that? Not speaking directly for David, but I believe the reason is for background kernel code which needs to do filesystem access during a thread's execution with *completely* different security context from that of the thread. Examples should be reasonably obvious; kNFSd is one, but it also includes anything where the kernel would poke directly into the filesystem, such as network filesystem cachefiles. Make it something sane, like a "struct nameidata" instead, and make it at least try to look like the path creation that is done by "open ()". Or create a "struct file *" or something. I can imagine having "mkdir()" being passed similar data as "open ()" (ie "lookup()"), but I cannot _possibly_ imagine it ever being valid to pass in something totally made-up to just mkdir(), and nothing else. There's something fundamentally wrong there. I would offer the suggestion of using the described "struct security" in-place in the task struct, in place of using all those fields individually. That would be, in effect the "default" security context for any given task, if "NULL" is passed to the appropriate vfs function. For CacheFiles and kNFSd, they could each allocate their own during initialization or new-connection and pass that to any "mkdir()", etc that they do on behalf of a given client. What makes mkdir() so magical? Also, what about all the other ops? Why is mkdir() special, but not "mknod()"? Why is "mkdir()" special, but not "rmdir()"? Really, none of this seems to make any sense unless you describe what is so magical about mkdir(). I think mkdir() was just an example he was using, probably because it was the first VFS call he needed to set a security context on. Next would come anything which CacheFiles or NFSd call on the underlying filesystem. Cheers, Kyle Moffett - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Thu, Aug 16, 2007 at 03:57:24PM -0700, Linus Torvalds wrote: > I personally consider this an affront to everythign that is decent. > > Why the *hell* would mkdir() be so magical as to need something like that? > > Make it something sane, like a "struct nameidata" instead, and make it at > least try to look like the path creation that is done by "open()". Or > create a "struct file *" or something. No. I agree that mkdir/mknod/etc. should all take the same thing. *However*, it should not be nameidata or file. Why? Because it should not contain vfsmount. Object creation should not *care* where fs is mounted. At all. The same goes for open(), BTW - letting it see the reference to vfsmount via nameidata is bloody wrong and I really couldn't care less what kinds of perversions apparmour crowd might like - pathnames simply do not belong there. Said that, I agree that we need uniform prototypes for all these guys and I can see arguments both for and against having creds passed by reference stored in whatever struct we are passing (for: copy-on-write refcounted cred objects would almost never have to be modified or copied and normally we would just pick the current creds reference from task_struct and assing it to a single field in whatever we pass instead of nameidata; against: might be conceptually simpler to have all that data directly in that struct and a helper filling it in). Which way we do that and which struct we end up passing is a very good question, but I want to make it very clear: having object creation/removal/ renaming methods[1] depend on which vfsmount we are dealing with is *wrong*. IOW, I strongly object against the use of nameidata here. - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Wed, 15 Aug 2007, David Howells wrote: > > Would you object greatly to functions like vfs_mkdir() gaining a security > parameter? What I'm thinking of is this: > > int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode, > struct security *security) I personally consider this an affront to everythign that is decent. Why the *hell* would mkdir() be so magical as to need something like that? Make it something sane, like a "struct nameidata" instead, and make it at least try to look like the path creation that is done by "open()". Or create a "struct file *" or something. I can imagine having "mkdir()" being passed similar data as "open()" (ie "lookup()"), but I cannot _possibly_ imagine it ever being valid to pass in something totally made-up to just mkdir(), and nothing else. There's something fundamentally wrong there. What makes mkdir() so magical? Also, what about all the other ops? Why is mkdir() special, but not "mknod()"? Why is "mkdir()" special, but not "rmdir()"? Really, none of this seems to make any sense unless you describe what is so magical about mkdir(). Linus - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Wednesday 15 August 2007 13:40, David Howells wrote:
>
> Hi Linus, Al,
>
> Would you object greatly to functions like vfs_mkdir() gaining a security
> parameter? What I'm thinking of is this:
>
> int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode,
> struct security *security)
>
> Where the security context is the state of the context at the time the call
> was issued:
>
> struct security {
> uid_t fsuid;
> git_t fsgid;
> struct group_info *group_info;
> void*security;
> struct key *session_keyring;
> struct key *process_keyring;
> struct key *thread_keyring;
>
> And perhaps:
>
> struct audit_context*audit_context;
> seccomp_t seccomp;
> };
>
> This would, for the most part, be a temporary affair, being set up by such
> as sys_mkdir()/sys_mkdirat() from data held in task_struct.
That's additional setup work unless that struct can be embedded in
task_struct. We would be complicating the common / fast / local case to
simplify the not-so-common case or cases.
-- Andreas
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Adding a security parameter to VFS functions
On Wednesday 15 August 2007 18:23, Casey Schaufler wrote: > > Hi Linus, Al, > > > > Would you object greatly to functions like vfs_mkdir() gaining a security > > parameter? > > Could you describe how this compares to the proposal that the > AppArmor developers suggested recently? I expect that we can > reduce the amount of discussion required, and maybe avoid some > confusion if you could do that. That's from one of those patches: -int vfs_mkdir(struct inode *dir, struct dentry *dentry, int mode) +int vfs_mkdir(struct inode *dir, struct dentry *dentry, struct vfsmount *mnt, + int mode) We need the vfsmount in the LSM hooks in addition to the dentry in order to figure out where in the filesystem namespace we are. The various vfs_ functions are the ones calling the LSM hooks. (The same could be achieved passing a struct path instead.) -- Andreas - To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] FS: file name should be unique in the same dir in procfs
Files name should be unique in the same directory.
Bug is reported here:
http://bugzilla.kernel.org/show_bug.cgi?id=8798
Signed-off-by: Zhang Rui <[EMAIL PROTECTED]>
---
fs/proc/generic.c |8
1 file changed, 8 insertions(+)
Index: linux-2.6.23-rc3/fs/proc/generic.c
===
--- linux-2.6.23-rc3.orig/fs/proc/generic.c
+++ linux-2.6.23-rc3/fs/proc/generic.c
@@ -523,6 +523,7 @@ static const struct inode_operations pro
static int proc_register(struct proc_dir_entry * dir, struct proc_dir_entry *
dp)
{
+ struct proc_dir_entry * tmp = NULL;
unsigned int i;
i = get_inode_number();
@@ -547,6 +548,13 @@ static int proc_register(struct proc_dir
}
spin_lock(&proc_subdir_lock);
+ for(tmp = dir->subdir; tmp; tmp = tmp->next)
+ if(!strcmp(dp->name, tmp->name)) {
+ spin_unlock(&proc_subdir_lock);
+ release_inode_number(i);
+ return -EEXIST;
+ }
+
dp->next = dir->subdir;
dp->parent = dir;
dir->subdir = dp;
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
