linux-kernel - Re: [PATCH 0/3] ramzswap: Eliminate stale data in compressed memory

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]

Date:	Wed, 5 May 2010 10:50:02 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Nitin Gupta <ngupta@...are.org>
cc:	Greg KH <greg@...ah.com>, Minchan Kim <minchan.kim@...il.com>,
	Pekka Enberg <penberg@...helsinki.fi>,
	Hugh Dickins <hugh.dickins@...cali.co.uk>,
	Cyp <cyp561@...il.com>, driverdev <devel@...verdev.osuosl.org>,
	linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/3] ramzswap: Eliminate stale data in compressed
 memory

On Wed, 5 May 2010, Nitin Gupta wrote:
> 
> Please see the original mail below (patch you nacked). Maybe, at that time, I didn't
> make it clear that ramzswap is really a *block device* :)

Oh, you're right. I looked up another patch of yours that had that 
swap_info" thing, and decided I hated that one with a passion, but it's 
not the one I NAK'ed in the earlier discussion.

Now that I see the block-layer patch, my reaction is (a) it's so much 
nicer than using that horrid nasty 'notifier' crap and (b) it reminds me 
why I wasn't entirely happy: it doesn't work - or even make sense - for 
filesystem-backed swap.

So when you do

	struct gendisk *disk = p->bdev->bd_disk;
	..
	if (disk->fops->swap_slot_free_notify)
		disk->fops->swap_slot_free_notify(p->bdev, offset);

there's nothing that says that 'offset' makes any sense at all, because if 
it's a swap-file on a device, it does all kinds of totally wrong things.

So I don't think that patch works either. I still suspect that the right 
"level" for something like this should be the 'mapping' level (which is 
how we actually do the write), but that seems to not work well with the 
block device layer.

So at a _minimum_, that 'disk->fops' approach needs to verify that the 
swap device is actually the whole bdev, and that the bdev isn't just the 
backing store for a swap _file_.

I dunno how to best check that. Either add a new flag to 
'swap_info_struct' that gets set on 'swapon()' whether it's a full device 
or a file. Or possibly just something like

	static int swap_is_block_device(struct swap_info_struct *p)
	{
		return S_ISBLK(p->swap_file->f_mapping->host);
	}

instead.

Because doing that 'disk->fops' thing _really_ isn't right if it isn't a 
disk.

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