linux-kernel - Re: [PATCH] Fix for bad lock balance in Containers

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] [day] [month] [year] [list]

Message-ID: <6599ad830706281609jebd82sdc1fb3ccc2d2df7b@mail.gmail.com>
Date:	Thu, 28 Jun 2007 19:09:05 -0400
From:	"Paul Menage" <menage@...gle.com>
To:	"Dhaval Giani" <dhaval@...ux.vnet.ibm.com>
Cc:	"Srivatsa Vaddagiri" <vatsa@...ux.vnet.ibm.com>,
	"Balbir Singh" <balbir@...ibm.com>, svaidy@...ux.vnet.ibm.com,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	bharata@...ux.vnet.ibm.com
Subject: Re: [PATCH] Fix for bad lock balance in Containers

On 6/26/07, Dhaval Giani <dhaval@...ux.vnet.ibm.com> wrote:
>
> There are a few questions I had with respect to the current code,
>
> Why is the increment of s_active dependent on the return value of
> simple_set_mnt?

I think it's because, as you observed, grab_super() is static and
hence not reachable from container.c. But I thought that the only side
effect of it that we needed (since we had a safe pointer to the
superblock not obtained via the superblock list) was the increment of
sb_active. (As it turns out I was wrong and we needed the s_umount
lock too).

Incrementing the sb_active count (if simple_set_mnt() failed) seemed
as though it would be wrong.

>
> What should be the correct approach to get the locking balance? As far
> as I can see, the correct method would be to call sget which would then
> correctly handle everything. But that would require a test function. I
> saw functionality similar to a test function in the beginning of
> container_get_sb(). Should that be seperated and put in a seperate test
> function so that sget can be called?

I don't quite remember why I did it this way originally, now. I've got
no objection to the code being cleaned up to fit the more common
approach if it's practical to do so, but it seems to me that your
current patch fix seems enough at least for now.

Thanks,

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