On Tue, Jan 09, 2007 at 02:42:03PM -0800, Andrew Morton wrote: > On Tue, 9 Jan 2007 16:23:37 -0600 > Michael Halcrow <[EMAIL PROTECTED]> wrote: > > > + page_virt = (char *)kmap(page); > > Do we _have_ to use kmap here? It's slow and theoretically deadlocky. > kmap_atomic() is much preferred. > > Can the other kmap() calls in ecryptfs be converted?
We will look into doing this. > We'd actually like to remove kmap() one day. Not much chance of that, but > it's an objective. > > > + if (!page_virt) { > > + rc = -ENOMEM; > > + printk(KERN_ERR "Error mapping page\n"); > > + goto out; > > + } > > + memset(page_virt, 0, PAGE_CACHE_SIZE); > > + if (page->index == 0) { > > + rc = ecryptfs_read_xattr_region( > > + page_virt, file->f_path.dentry); > > Are we assured that ecryptfs_read_xattr_region() cannot overrun the > page? Yes: --- int ecryptfs_read_xattr_region(char *page_virt, struct dentry*ecryptfs_dentry) { ssize_t size; int rc = 0; size = ecryptfs_getxattr(ecryptfs_dentry, ECRYPTFS_XATTR_NAME, page_virt, ECRYPTFS_DEFAULT_EXTENT_SIZE); --- That winds up calling the lower filesystem's getxattr with ECRYPTFS_DEFAULT_EXTENT_SIZE as the size parameter. eCryptfs validates this value against PAGE_CACHE_SIZE in main.c::ecryptfs_init(). > > + set_header_info(page_virt, crypt_stat); > > + } > > The kernel must always run flush_dcache_page() after modifying a pagecache > page by hand. Please review all of ecryptfs for this. We will work on some patches to address these issues. Mike - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/