linux-kernel - Re: [PATCH] proc: calculate the correct /proc/<pid> link count

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: <m1abi1tt5h.fsf@frodo.ebiederm.org>
Date:	Wed, 04 Jun 2008 03:02:18 -0700
From:	ebiederm@...ssion.com (Eric W. Biederman)
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Vegard Nossum <vegard.nossum@...il.com>,
	Pavel Emelyanov <xemul@...nvz.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] proc: calculate the correct /proc/<pid> link count

Andrew Morton <akpm@...ux-foundation.org> writes:

> On Tue, 3 Jun 2008 15:16:59 +0200 Vegard Nossum <vegard.nossum@...il.com> wrote:
>
>> From: Vegard Nossum <vegard.nossum@...il.com>
>> Date: Mon, 2 Jun 2008 08:57:45 +0200
>> Subject: [PATCH] proc: calculate the correct /proc/<pid> link count
>> 
>> commit e9720acd728a46cb40daa52c99a979f7c4ff195c

I sent a message acking the patch but it seems to have gotten lost.
Acked-by: "Eric W. Biederman" <ebiederm@...ssion.com>

>> +static unsigned int pid_entry_count_dirs(const struct pid_entry *entries,
>> +	unsigned int n)
>> +{
>> +	unsigned int i;
>> +	unsigned int count;
>> +
>> +	count = 0;
>> +	for (i = 0; i < n; ++i) {
>> +		if (S_ISDIR(entries[i].mode))
>> +			++count;
>> +	}
>> +
>> +	return count;
>> +}
>
> I'm unable to correlate the code with the comment.  There is nothing in
> here which handles .  and ..?

Because they don't appear in the table.  It seems the comment was to make
that clear.

>> @@ -2585,10 +2602,9 @@ static struct dentry *proc_pid_instantiate(struct inode
> *dir,
>>  	inode->i_op = &proc_tgid_base_inode_operations;
>>  	inode->i_fop = &proc_tgid_base_operations;
>>  	inode->i_flags|=S_IMMUTABLE;
>> -	inode->i_nlink = 5;
>> -#ifdef CONFIG_SECURITY
>> -	inode->i_nlink += 1;
>> -#endif
>> +
>> +	inode->i_nlink = 2 + pid_entry_count_dirs(tgid_base_stuff,
>> +		ARRAY_SIZE(tgid_base_stuff));
>>  
>
> oh, can we do that?  Is it possible for some code somewhere to come
> along and add a new entry to /proc/pid which doesn't appear in
> these static tables?

Currently we do not dynamically modify the pid_entry tables.
On some days I think it would be a nice addition, to make it easier to
handle modular subsystems.  Given the number of #ifdefs we have
in those tables something more dynamic may ultimately be the way
to go.

However we still have in lookup:
	/*
	 * Yes, it does not scale. And it should not. Don't add
	 * new entries into /proc/<tgid>/ without very good reasons.
	 */
Which doesn't seem to have much impact as these directories are
slowly growing.


> I guess that doesn't happen.  In which case can we not calculate the
> unmber of directories in these two tables just a single time, at
> bootup?
>
> I think I'm missing things here...

Not much.  

Historically /proc used to be very bad with the link counts on
directories.   There was a switch statement that hard coded nlinks for
every directory, and only used the values 2 or 3.  Last time I was in
there I fixed it up so we actually returned the proper hard link
counts for the directories.  In this last conversation I realized we
could be more maintainable without a hard coded number.

Eric

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