Bug#714974: [PATCH] jfs: avoid misuse of cookie value of 2
On Wed, Aug 14, 2013 at 10:54:31PM -0500, Dave Kleikamp wrote:
> For the sake of those not watching
> https://bugzilla.kernel.org/show_bug.cgi?id=60737
>
> It looks like the problem is that jfs was using a cookie value of 2 for
> a real directory entry, where NFSv4 expect 2 to represent "..". This
> patch has so far only been lightly tested.
>
> NFSv4 reserves cookie values 0, 1 and 2 for a rewind, and the "." and ".."
> entries. jfs was using 0 and 1 for "." and "..", but 2 for a regular entry.
> This patch makes jfs conform by using 1 and 2 for "." and ".." and fixes
> any regular entry using the value 2.
Oh, I'd forgotten that. From rfc 5661:
For some file system environments, the directory entries "." and
".." have special meaning, and in other environments, they do
not. If the server supports these special entries within a
directory, they SHOULD NOT be returned to the client as part of
the READDIR response. To enable some client environments, the
cookie values of zero, 1, and 2 are to be considered reserved.
Note that the UNIX client will use these values when combining
the server's response and local representations to enable a
fully formed UNIX directory presentation to the application.
OK!
--b.
>
> Signed-off-by: Dave Kleikamp <dave.kleikamp@oracle.com>
>
> diff --git a/fs/jfs/jfs_dtree.c b/fs/jfs/jfs_dtree.c
> index 8743ba9..93466e8 100644
> --- a/fs/jfs/jfs_dtree.c
> +++ b/fs/jfs/jfs_dtree.c
> @@ -349,11 +349,8 @@ static u32 add_index(tid_t tid, struct inode *ip, s64 bn, int slot)
>
> ASSERT(DO_INDEX(ip));
>
> - if (jfs_ip->next_index < 2) {
> - jfs_warn("add_index: next_index = %d. Resetting!",
> - jfs_ip->next_index);
> - jfs_ip->next_index = 2;
> - }
> + if (jfs_ip->next_index < 3) {
> + jfs_ip->next_index = 3;
>
> index = jfs_ip->next_index++;
>
> @@ -2864,7 +2861,7 @@ void dtInitRoot(tid_t tid, struct inode *ip, u32 idotdot)
> } else
> ip->i_size = 1;
>
> - jfs_ip->next_index = 2;
> + jfs_ip->next_index = 3;
> } else
> ip->i_size = IDATASIZE;
>
> @@ -2951,7 +2948,7 @@ static void add_missing_indices(struct inode *inode, s64 bn)
> for (i = 0; i < p->header.nextindex; i++) {
> d = (struct ldtentry *) &p->slot[stbl[i]];
> index = le32_to_cpu(d->index);
> - if ((index < 2) || (index >= JFS_IP(inode)->next_index)) {
> + if ((index < 3) || (index >= JFS_IP(inode)->next_index)) {
> d->index = cpu_to_le32(add_index(tid, inode, bn, i));
> if (dtlck->index >= dtlck->maxcnt)
> dtlck = (struct dt_lock *) txLinelock(dtlck);
> @@ -3031,7 +3028,7 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
> struct jfs_dirent *jfs_dirent;
> int jfs_dirents;
> int overflow, fix_page, page_fixed = 0;
> - static int unique_pos = 2; /* If we can't fix broken index */
> + static int unique_pos = 3; /* If we can't fix broken index */
>
> if (ctx->pos == DIREND)
> return 0;
> @@ -3039,15 +3036,16 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
> if (DO_INDEX(ip)) {
> /*
> * persistent index is stored in directory entries.
> - * Special cases: 0 = .
> - * 1 = ..
> + * Special cases: 0 = rewind
> + * 1 = .
> + * 2 = ..
> * -1 = End of directory
> */
> do_index = 1;
>
> dir_index = (u32) ctx->pos;
>
> - if (dir_index > 1) {
> + if (dir_index > 2) {
> struct dir_table_slot dirtab_slot;
>
> if (dtEmpty(ip) ||
> @@ -3090,18 +3088,18 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
> return 0;
> }
> } else {
> - if (dir_index == 0) {
> + if (dir_index < 2) {
> /*
> * self "."
> */
> - ctx->pos = 0;
> + ctx->pos = 1;
> if (!dir_emit(ctx, ".", 1, ip->i_ino, DT_DIR))
> return 0;
> }
> /*
> * parent ".."
> */
> - ctx->pos = 1;
> + ctx->pos = 2;
> if (!dir_emit(ctx, "..", 2, PARENT(ip), DT_DIR))
> return 0;
>
> @@ -3122,22 +3120,24 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
> /*
> * Legacy filesystem - OS/2 & Linux JFS < 0.3.6
> *
> - * pn = index = 0: First entry "."
> - * pn = 0; index = 1: Second entry ".."
> + * pn = 0; index = 1: First entry "."
> + * pn = 0; index = 2: Second entry ".."
> * pn > 0: Real entries, pn=1 -> leftmost page
> * pn = index = -1: No more entries
> */
> dtpos = ctx->pos;
> - if (dtpos == 0) {
> + if (dtpos < 2) {
> + ctx->pos = 1;
> /* build "." entry */
> if (!dir_emit(ctx, ".", 1, ip->i_ino, DT_DIR))
> return 0;
> - dtoffset->index = 1;
> + dtoffset->index = 2;
> ctx->pos = dtpos;
> }
>
> if (dtoffset->pn == 0) {
> - if (dtoffset->index == 1) {
> + if (dtoffset->index == 2) {
> + ctx->pos = 2;
> /* build ".." entry */
> if (!dir_emit(ctx, "..", 2, PARENT(ip), DT_DIR))
> return 0;
> @@ -3210,8 +3210,12 @@ int jfs_readdir(struct file *file, struct dir_context *ctx)
> * directory index for the lost+found
> * directory. Rather than let it go,
> * we can try to fix it.
> + *
> + * Additionally, a value of 2 used to be
> + * valid, but it didn't work well with
> + * NFSv4, so if found, we need to change it
> */
> - if ((jfs_dirent->position < 2) ||
> + if ((jfs_dirent->position < 3) ||
> (jfs_dirent->position >=
> JFS_IP(ip)->next_index)) {
> if (!page_fixed && !isReadOnly(ip)) {
Reply to: