Re: smbfs Oops with Linux 2.6.3

[Zwane Mwaikambo]

On Wed, 10 Mar 2004, Urban Widmark wrote:

> On Tue, 9 Mar 2004, Linus Torvalds wrote:
>
> > As to how something like that could happen, I have absolutely no clue. The
> > "smb_install_null_ops()" would seem to cause that, but that's all I can
> > say.
> >
> > Maybe the "smp_ops_null" thing should be filled in with stuff that always
> > returns EINVAL or something? Rather than actual NULL pointers that will
> > oops if they are ever used?

I originally didn't fill them all in intentionally, doing so may be best.

> smp_ops_null should really make all functions block and wait for the
> connection to be created and given to us from smbmount. That would make it
> behave more like smbfs in 2.4 does.
>
> I am thinking of something like this for each entry in smp_ops_null.
>
> int whatever_it_is_that_I_am(args)
> {
> 	timeleft = wait_event_interruptible_timeout(...)
> 	if (!timeleft || signal_pending(current))
> 		return -EIO;
> 	if (!server->ops->whatever_it_is_that_I_am)
> 		return -EIO;
> 	return server->ops->whatever_it_is_that_I_am(args)
> }

Thanks Urban, i have posted the following on bugzilla
(http://bugzilla.kernel.org/show_bug.cgi?id=1671) for testing. But,
it appears racy wrt getattr and win9x servers.

Index: linux-2.6.4-rc3/fs/smbfs/proc.c
===================================================================
RCS file: /home/cvsroot/linux-2.6.4-rc3/fs/smbfs/proc.c,v
retrieving revision 1.1.1.1
diff -u -p -B -r1.1.1.1 proc.c
--- linux-2.6.4-rc3/fs/smbfs/proc.c	10 Mar 2004 01:05:32 -0000	1.1.1.1
+++ linux-2.6.4-rc3/fs/smbfs/proc.c	10 Mar 2004 17:16:22 -0000
@@ -56,6 +56,7 @@ static struct smb_ops smb_ops_os2;
 static struct smb_ops smb_ops_win95;
 static struct smb_ops smb_ops_winNT;
 static struct smb_ops smb_ops_unix;
+static struct smb_ops smb_ops_null;

 static void
 smb_init_dirent(struct smb_sb_info *server, struct smb_fattr *fattr);
@@ -2794,10 +2795,46 @@ out:
 }

 static int
+smb_proc_ops_wait(struct smb_sb_info *server)
+{
+	int result;
+	DECLARE_WAIT_QUEUE_HEAD(wq);
+
+	result = wait_event_interruptible_timeout(wq,
+			server->ops != &smb_ops_null, 5*HZ);
+
+	if (!result || signal_pending(current))
+		return -EIO;
+
+	return 0;
+}
+
+static int
 smb_proc_getattr_null(struct smb_sb_info *server, struct dentry *dir,
-		      struct smb_fattr *attr)
+			  struct smb_fattr *fattr)
 {
-	return -EIO;
+	int result;
+
+	if (smb_proc_ops_wait(server) < 0)
+		return -EIO;
+
+	smb_init_dirent(server, fattr);
+	result = server->ops->getattr(server, dir, fattr);
+	smb_finish_dirent(server, fattr);
+
+	return result;
+}
+
+static int
+smb_proc_readdir_null(struct file *filp, void *dirent, filldir_t filldir,
+		      struct smb_cache_control *ctl)
+{
+	struct smb_sb_info *server = server_from_dentry(filp->f_dentry);
+
+	if (smb_proc_ops_wait(server) < 0)
+		return -EIO;
+
+	return server->ops->readdir(filp, dirent, filldir, ctl);
 }

 int
@@ -3431,6 +3467,7 @@ static struct smb_ops smb_ops_unix =
 /* Place holder until real ops are in place */
 static struct smb_ops smb_ops_null =
 {
+	.readdir	= smb_proc_readdir_null,
 	.getattr	= smb_proc_getattr_null,
 };

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/