Neil Brown wrote:
- There is no upper bound imposed on the size of the cache, and no way
for memory pressure to shrink the cache except indirectly by
discarding inodes.
I cannot see this being exploitable as getting access to lots of
credentials would be hard for any given user. However I feel that
some cleaning might be in order.
I guess there is no upper bound checking because I didn't see any type
of boundary checking the server hashing code I stoled 8-) Or maybe
I just missed it... Is there an example and what would trigger
this cleanup?
- The nfs_zap_access_cache call isn't cheap. If that could be
amortised somehow it would be good. Maybe use some version tagging
so that when an inode is reused the access entry will no longer
match in some way. Then just clean the table by periodic scans that
discard based on timeout information ?
yes.. I did realize that ifs_zap_access_cache would be expensive...
and I think there might be an issue with holding the access_hash lock
spin lock while calling put_rpccred() since it can also take out
another spin lock... Maybe I just spin through the table marking
entries for deletion and then let somebody else clean them up??
Is there already a clean up process that I would us? I don't
recall one...
It occurs to me that the large majority of inodes will only be
accessed by a single user and so need not reside in the cache.
So how about having a single "struct nfs_access_entry" pointer in the
inode.
When we do a lookup we look there first.
When we want to add an entry, we try to add it there first.
When we end up with two current access entries for the same inode,
only then do we insert them into the hash.
To rephrase to make sure I understand....
1) P1(uid=1) creates an access pointer in the nfs_inode
2) P2(uid=2) sees the access pointer is not null so it adds them both
to the table, right?
We would need to be able to tell from the inode whether anything is
hashed or not. This could simply be if the nfs_access_entry point is
non-null, and its hashlist it non-empty. Or we could just use a bit
flag somewhere.
So I guess it would be something like:
if (nfs_inode->access == null)
set nfs_inode->access
if (nfs_inode->access =! NULL && nfs_inode->access_hash == empty)
move both pointer into hast able.
if (nfs_inode->access == null && nfs_inode->access_hash != empty)
use hastable.
But now the question is how would I know when there is only one
entry in the table? Or do we just let the hash table "drain"
naturally and when it become empty we start with the nfs_inode->access
pointer again... Is this close to what your thinking??
steved.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
