linux-kernel - Re: [PATCH RESEND] cifs: file: initialize oparms.reconnect before using it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20130729163532.GA4254@jack.whiskey>
Date:	Mon, 29 Jul 2013 18:35:32 +0200
From:	Andi Shyti <andi@...zian.org>
To:	Jeff Layton <jlayton@...hat.com>
Cc:	Andi Shyti <andi@...zian.org>, smfrench@...il.com,
	linux-cifs@...r.kernel.org, linux-kernel@...r.kernel.org,
	mikko.rapeli@....fi, pshilovsky@...ba.org
Subject: Re: [PATCH RESEND] cifs: file: initialize oparms.reconnect before
 using it

> > +	oparms.tcon = tcon;
> > +	oparms.cifs_sb = cifs_sb;
> > +	oparms.desired_access = desired_access;
> > +	oparms.create_options = create_options;
> 
> This patch just moves the brokenness around. You're
> setting .desired_access here to an unintialized variable.
> create_options also looks like it may potentially be wrong at this
> point.

Urrrca! This is what I achieve when I do one last fix before
going to sleep.

I spent a bit of time more going through the cifs/smb code and
the most sensful fix looks this

diff --git a/fs/cifs/file.c b/fs/cifs/file.c
index 1e57f36..7e36ae3 100644
--- a/fs/cifs/file.c
+++ b/fs/cifs/file.c
@@ -647,6 +647,7 @@ cifs_reopen_file(struct cifsFileInfo *cfile, bool can_flu
                                     oflags, &oplock, &cfile->fid.netfid, xid
                if (rc == 0) {
                        cifs_dbg(FYI, "posix reopen succeeded\n");
+                       oparms.reconnect = true;
                        goto reopen_success;
                }
                /*

There is only one case when reconnect becames false, that is when
open = smb2_open_file and calls SMB2_open() that calls
add_durable_context() that sets reconnect = false with some
nested ifs in between, and still only in case
everything succeeds. We are checking reconnect only for this
case, otherwise we could get rid of the if (oparms.reconnect) and
not falling into the unknown state.

If it makes sense, I can send the above suggestion.

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