linux-kernel - Re: [PATCH] loop: fput() called in loop_clr_fd() may cause bd

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]

Date:	Sun, 18 Dec 2011 01:37:45 +0300
From:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>
To:	Al Viro <viro@...IV.linux.org.uk>
Cc:	Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
	Jens Axboe <axboe@...nel.dk>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kay Sievers <kay.sievers@...y.org>,
	Namhyung Kim <namhyung@...il.com>,
	Lukas Czerner <lczerner@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] loop: fput() called in loop_clr_fd() may cause bd_mutex
 recursive locking

On (12/17/11 22:30), Al Viro wrote:
> > Sorry, why is that a false positive?
> > 
> > blkdev_put() calls lo_release() while holding bd_mutex,
> > lo_release() calls loop_clr_fd() -> fput(). fput() once again
> > attempts to grub already held bd_mutex calling blkdev_put().
> > Looks like a recursion to me.
> 
> Because of this:
>         /* Avoid recursion */
>         f = file;
>         while (is_loop_device(f)) {
>                 struct loop_device *l;
> 
>                 if (f->f_mapping->host->i_bdev == bdev)
>                         goto out_putf;
> 
>                 l = f->f_mapping->host->i_bdev->bd_disk->private_data;
>                 if (l->lo_state == Lo_unbound) {
>                         error = -EINVAL;
>                         goto out_putf;
>                 }
>                 f = l->lo_backing_file;
>         }
> in loop_set_fd().  

Oh, thanks. I didn't notice that one.

> Think of it for a minute - if we could run into the
> same bdev in that recursion, what would have happened on read() from
> that sucker? So yes, it is a false positive. 

I've tried read()/write() some time ago and it worked. Perhaps, I just
wasn't "lucky" enough to hit any problems.

> And your patch would simply leave the underlying device opened,
> with all the consequences...
> 
well, that sucks.


	Sergey
--
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/