Good catch - merged into cifs-2.6.git for-next
In one of the cases we also as an alternative could have skipped the
get_xid instead as an alternative (and passed zero as xid to negotiate
in that case) - but your approach may be slightly better
On Mon, Oct 17, 2022 at 8:42 AM Zhang Xiaoxu <zhangxiaoxu5@xxxxxxxxxx> wrote:
>
> Found some xid leak with the following cocci script:
>
> /usr/bin/spatch -I include -timeout 60 -very_quiet \
> -sp_file missing-free_xid.cocci fs/cifs
>
> @r1@
> identifier xid;
> position p;
> @@
> ...
> xid = get_xid();
> <+... when != free_xid(xid)
> if (...) {
> ... when != free_xid(xid)
> when forall
> return@p ...;
> }
> ...+>
> free_xid(xid);
>
> @depends on r1@
> position r1.p;
> @@
> + free_xid(xid);
> return@p ...;
>
> @r2@
> identifier xid;
> position p;
> @@
> ...
> unsigned int xid = get_xid();
> <+... when != free_xid(xid)
> if (...) {
> ... when != free_xid(xid)
> when forall
> return@p ...;
> }
> ...+>
> free_xid(xid);
>
> @depends on r2@
> position r2.p;
> @@
> + free_xid(xid);
> return@p ...;
>
> @r3@
> identifier xid;
> position p;
> @@
> ...
> xid = get_xid();
> ... when != \(free_xid\|_free_xid\)(xid);
> return@p ...;
>
> @depends on r3@
> position r3.p;
> @@
> + free_xid(xid);
> return@p ...;
>
> @r4@
> identifier xid;
> position p;
> @@
> ...
> unsigned int xid = get_xid();
> ... when != \(free_xid\|_free_xid\)(xid);
> return@p ...;
>
> @depends on r4@
> position r4.p;
> @@
> + free_xid(xid);
> return@p ...;
>
> Zhang Xiaoxu (5):
> cifs: Fix xid leak in cifs_create()
> cifs: Fix xid leak in cifs_copy_file_range()
> cifs: Fix xid leak in cifs_flock()
> cifs: Fix xid leak in cifs_ses_add_channel()
> cifs: Fix xid leak in cifs_get_file_info_unix()
>
> fs/cifs/cifsfs.c | 7 +++++--
> fs/cifs/dir.c | 6 ++++--
> fs/cifs/file.c | 11 +++++++----
> fs/cifs/inode.c | 6 ++++--
> fs/cifs/sess.c | 1 +
> 5 files changed, 21 insertions(+), 10 deletions(-)
>
> --
> 2.31.1
>
--
Thanks,
Steve
