#4538 closed defect (invalid)

mutex is not initilaized in jffs2_read_inode

Reported by: chenjin_zhong Owned by:
Priority: normal Milestone: 5.1
Component: fs/jffs2 Version: 5
Severity: normal Keywords:
Cc: Blocked By:
Blocking:

Description

HI, I have found when call jffs2_read_inode to read inode. the f->sem is not initiliazed and locked, but it is be unlocked. The peice of source code is as follows:
static int jffs2_read_inode (struct _inode *inode)
{

struct jffs2_inode_info *f;
struct jffs2_sb_info *c;
struct jffs2_raw_inode latest_node;
int ret;

D1(printk(KERN_DEBUG "jffs2_read_inode(): inode->i_ino == %lu\n", inode->i_ino));

f = JFFS2_INODE_INFO(inode);
c = JFFS2_SB_INFO(inode->i_sb);

jffs2_init_inode_info(f);
ret = jffs2_do_read_inode(c, f, inode->i_ino, &latest_node);

if (ret) {

mutex_unlock(&f->sem);
return ret;

}


inode->i_mode = jemode_to_cpu(latest_node.mode);
inode->i_uid = je16_to_cpu(latest_node.uid);
inode->i_gid = je16_to_cpu(latest_node.gid);
inode->i_size = je32_to_cpu(latest_node.isize);
inode->i_atime = je32_to_cpu(latest_node.atime);
inode->i_mtime = je32_to_cpu(latest_node.mtime);
inode->i_ctime = je32_to_cpu(latest_node.ctime);

inode->i_nlink = f->inocache->pino_nlink;
mutex_unlock(&f->sem);


D1(printk(KERN_DEBUG "jffs2_read_inode() returning\n"));
return 0;

}

Change History (6)

comment:1 Changed on 10/28/21 at 05:20:50 by Sebastian Huber

Component: adminfs/jaffs2
Resolution: invalid
Status: newclosed

Thanks for your interest in the RTEMS port of JFFS2. If you have questions, then you could also ask them on the devel@… mailing list. The RTEMS port of JFFS2 does not use a file system internal locking. There is a global lock for each JFFS2 instance:

static void rtems_jffs2_do_lock(struct super_block *sb)
{
        rtems_recursive_mutex_lock(&sb->s_mutex);
}

static void rtems_jffs2_do_unlock(struct super_block *sb)
{
        rtems_recursive_mutex_unlock(&sb->s_mutex);
}

comment:2 in reply to:  1 Changed on 10/28/21 at 14:40:25 by chenjin_zhong

Resolution: invalid
Status: closedreopened

Replying to Sebastian Huber:

Thanks for your interest in the RTEMS port of JFFS2. If you have questions, then you could also ask them on the devel@… mailing list. The RTEMS port of JFFS2 does not use a file system internal locking. There is a global lock for each JFFS2 instance:

static void rtems_jffs2_do_lock(struct super_block *sb)
{
        rtems_recursive_mutex_lock(&sb->s_mutex);
}

static void rtems_jffs2_do_unlock(struct super_block *sb)
{
        rtems_recursive_mutex_unlock(&sb->s_mutex);
}

I have compared it with Linux JFFS2. The peice of source code in Linux is as follows,As shown in black-body section, the f->sem is not be initialized and locked in RTEMS.

struct jffs2_sb_info *c;
struct jffs2_raw_inode latest_node;
union jffs2_device_node jdev;
struct inode *inode;
dev_t rdev = 0;
int ret;
jffs2_dbg(1, "%s(): ino == %lu\n", func, ino);
inode = iget_locked(sb, ino);
if (!inode)
return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW))
return inode;
f = JFFS2_INODE_INFO(inode);
c = JFFS2_SB_INFO(inode->i_sb);
jffs2_init_inode_info(f);
mutex_lock(&f->sem);
ret = jffs2_do_read_inode(c, f, inode->i_ino, &latest_node);
if (ret)
goto error;
inode->i_mode = jemode_to_cpu(latest_node.mode);
i_uid_write(inode, je16_to_cpu(latest_node.uid));
i_gid_write(inode, je16_to_cpu(latest_node.gid));
inode->i_size = je32_to_cpu(latest_node.isize);
inode->i_atime = ITIME(je32_to_cpu(latest_node.atime));
inode->i_mtime = ITIME(je32_to_cpu(latest_node.mtime));
inode->i_ctime = ITIME(je32_to_cpu(latest_node.ctime));
set_nlink(inode, f->inocache->pino_nlink);
inode->i_blocks = (inode->i_size + 511) >> 9;

comment:3 Changed on 10/28/21 at 14:43:37 by Sebastian Huber

Resolution: invalid
Status: reopenedclosed

Yes, the f->sem is an empty structure, it is not initialized, it is not used, locked or whatever in RTEMS. This is intentional and not a bug. I repeat: The RTEMS port of JFFS2 does not use a file system internal locking. There is a global lock for each JFFS2 instance.

comment:4 in reply to:  3 ; Changed on 10/28/21 at 15:05:51 by chenjin_zhong

Replying to Sebastian Huber:

Yes, the f->sem is an empty structure, it is not initialized, it is not used, locked or whatever in RTEMS. This is intentional and not a bug. I repeat: The RTEMS port of JFFS2 does not use a file system internal locking. There is a global lock for each JFFS2 instance.

Thank you! I got it. but we Suppose that a GC thread or other thread/task and main task calls jffs2_do_readinode simultaneous. a global lock for each JFFS2 instance can't work.

comment:5 Changed on 10/28/21 at 15:07:51 by chenjin_zhong

Resolution: invalid
Status: closedreopened

comment:6 in reply to:  4 Changed on 10/28/21 at 18:26:10 by Sebastian Huber

Resolution: invalid
Status: reopenedclosed

Replying to chenjin_zhong:

Replying to Sebastian Huber:

Yes, the f->sem is an empty structure, it is not initialized, it is not used, locked or whatever in RTEMS. This is intentional and not a bug. I repeat: The RTEMS port of JFFS2 does not use a file system internal locking. There is a global lock for each JFFS2 instance.

Thank you! I got it. but we Suppose that a GC thread or other thread/task and main task calls jffs2_do_readinode simultaneous. a global lock for each JFFS2 instance can't work.

It is not a high performance approach, but it works. See testsuites/fstests/fsjffs2gc01/init.c.

Note: See TracTickets for help on using tickets.