Skip to contentSkip to conversation

seq_file:update file->f_pos when lseek() to m->read_pos

PATCH In review Jiaxing Wang 5 participants 5 messages Jun 29, 2013 – Jul 4, 2013 VFS

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 seq_file:update file->f_pos when lseek() to m->read_pos

After pread(), file->f_pos and m->read_pos get different, and lseek() to m->read_pos did not update file->f_pos, then a subsequent read may read from a wrong position, the following program shows the problem:

    char str1[32] = { 0 };
    char str2[32] = { 0 };
    int poffset = 10;
    int count = 20;

    /*open any seq file*/
    int fd = open("/proc/modules", O_RDONLY);

    pread(fd, str1, count, poffset);
    printf("pread:%s\n", str1);

    /*seek to where m->read_pos is*/
    lseek(fd, poffset+count, SEEK_SET);

    /*supposed to read from poffset+count, but this read from position 0*/
    read(fd, str2, count);
    printf("read:%s\n", str2);
---
 fs/seq_file.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
fs/seq_file.c +2 -1
index 774c1eb..4b22b26 100644
@@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence)
 				m->read_pos = offset;
 				retval = file->f_pos = offset;
 			}
-		}
+		} else
+			file->f_pos = offset;
 	}
 	file->f_version = m->version;
 	mutex_unlock(&m->lock);
Signature · 1 line
1.8.1.2
#1 lore raw
Quoted 34 lines · Jiaxing Wang

After pread(), file->f_pos and m->read_pos get different, and lseek() to m->read_pos did not update file->f_pos, then a subsequent read may read from a wrong position, the following program shows the problem:

     char str1[32] = { 0 };
     char str2[32] = { 0 };
     int poffset = 10;
     int count = 20;

     /*open any seq file*/
     int fd = open("/proc/modules", O_RDONLY);

     pread(fd, str1, count, poffset);
     printf("pread:%s\n", str1);

     /*seek to where m->read_pos is*/
     lseek(fd, poffset+count, SEEK_SET);

     /*supposed to read from poffset+count, but this read from position 0*/
     read(fd, str2, count);
     printf("read:%s\n", str2);
---
  fs/seq_file.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
fs/seq_file.c +0 -0
index 774c1eb..4b22b26 100644

fs/seq_file.c loff_t seq_lseek(struct file *file, loff_t offset, int whence) · line 335

Quoted 9 lines
fs/seq_file.c +2 -1
@@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence)
  				m->read_pos = offset;
  				retval = file->f_pos = offset;
  			}
-		}
+		} else
+			file->f_pos = offset;
  	}
  	file->f_version = m->version;
  	mutex_unlock(&m->lock);

This does not appear to be a problem, in linux man page, the behaver seems clearly defined:

DESCRIPTION
pread() reads up to count bytes from file descriptor fd at
offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed.

pwrite() writes up to count bytes from the buffer starting at
buf to the file descriptor fd at offset offset. The file offset is not changed.

#2 lore raw
Quoted 46 lines · fangdong
Quoted 34 lines · Jiaxing Wang

After pread(), file->f_pos and m->read_pos get different, and lseek() to m->read_pos did not update file->f_pos, then a subsequent read may read from a wrong position, the following program shows the problem:

     char str1[32] = { 0 };
     char str2[32] = { 0 };
     int poffset = 10;
     int count = 20;

     /*open any seq file*/
     int fd = open("/proc/modules", O_RDONLY);

     pread(fd, str1, count, poffset);
     printf("pread:%s\n", str1);

     /*seek to where m->read_pos is*/
     lseek(fd, poffset+count, SEEK_SET);

     /*supposed to read from poffset+count, but this read from position 0*/
     read(fd, str2, count);
     printf("read:%s\n", str2);
---
  fs/seq_file.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
fs/seq_file.c +0 -0
index 774c1eb..4b22b26 100644

fs/seq_file.c loff_t seq_lseek(struct file *file, loff_t offset, int whence) · line 335

Quoted 9 lines
fs/seq_file.c +2 -1
@@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence)
                  m->read_pos = offset;
                  retval = file->f_pos = offset;
              }
-        }
+        } else
+            file->f_pos = offset;
      }
      file->f_version = m->version;
      mutex_unlock(&m->lock);

This does not appear to be a problem, in linux man page, the behaver seems clearly defined:

Quoted 10 lines

DESCRIPTION
pread() reads up to count bytes from file descriptor fd at offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed.

pwrite() writes up to count bytes from the buffer starting at buf to the file descriptor fd at offset offset. The file offset is not changed.

Signature · 4 lines
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/

There's no problem that pread() don't change file->f_pos, but I think lseek() should have changed it. Any comments from other people, and Alexander?

#3 lore raw

Hi Jiaxing,
Please refer to inline comment.:)

Quoted 50 lines · Jiaxing Wang
Quoted 49 lines · fangdong
Quoted 34 lines · Jiaxing Wang

After pread(), file->f_pos and m->read_pos get different, and lseek() to m->read_pos did not update file->f_pos, then a subsequent read may read from a wrong position, the following program shows the problem:

     char str1[32] = { 0 };
     char str2[32] = { 0 };
     int poffset = 10;
     int count = 20;

     /*open any seq file*/
     int fd = open("/proc/modules", O_RDONLY);

     pread(fd, str1, count, poffset);
     printf("pread:%s\n", str1);

     /*seek to where m->read_pos is*/
     lseek(fd, poffset+count, SEEK_SET);

     /*supposed to read from poffset+count, but this read from position 0*/
     read(fd, str2, count);
     printf("read:%s\n", str2);
---
  fs/seq_file.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
fs/seq_file.c +0 -0
index 774c1eb..4b22b26 100644

fs/seq_file.c loff_t seq_lseek(struct file *file, loff_t offset, int whence) · line 335

Quoted 9 lines
fs/seq_file.c +2 -1
@@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence)
                  m->read_pos = offset;
                  retval = file->f_pos = offset;
              }
-        }
+        } else
+            file->f_pos = offset;
      }
      file->f_version = m->version;
      mutex_unlock(&m->lock);

This does not appear to be a problem, in linux man page, the behaver seems clearly defined:

DESCRIPTION
pread() reads up to count bytes from file descriptor fd at offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed.

Quoted 8 lines
Quoted 7 lines

pwrite() writes up to count bytes from the buffer starting at buf to the file descriptor fd at offset offset. The file offset is not changed.

Signature · 4 lines
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/

There's no problem that pread() don't change file->f_pos, but I think lseek() should have changed it.

No, the way that seq_seek deal with pos is right. Remember this seq file, it is an iterator interface, e.g. it provides infos of each element of a list, so the we should make sure that the info we read from seq file is the whole infos of each entry, so seq_lseek can not set pos to a optional point, only can on the points that divided by a whole infos size of each entry. If we can not set pos on the special position, ste it as 0, I think it's reasonable.

Thanks,
Gu

Quoted 7 lines

Any comments from other people, and Alexander?
--
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/

#4 lore raw
In reply to Jiaxing Wang
Quoted 34 lines · Jiaxing Wang

After pread(), file->f_pos and m->read_pos get different, and lseek() to m->read_pos did not update file->f_pos, then a subsequent read may read from a wrong position, the following program shows the problem:

     char str1[32] = { 0 };
     char str2[32] = { 0 };
     int poffset = 10;
     int count = 20;

     /*open any seq file*/
     int fd = open("/proc/modules", O_RDONLY);

     pread(fd, str1, count, poffset);
     printf("pread:%s\n", str1);

     /*seek to where m->read_pos is*/
     lseek(fd, poffset+count, SEEK_SET);

     /*supposed to read from poffset+count, but this read from position 0*/
     read(fd, str2, count);
     printf("read:%s\n", str2);
---
  fs/seq_file.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)
fs/seq_file.c +0 -0
index 774c1eb..4b22b26 100644

fs/seq_file.c loff_t seq_lseek(struct file *file, loff_t offset, int whence) · line 335

Quoted 9 lines
fs/seq_file.c +2 -1
@@ -328,7 +328,8 @@ loff_t seq_lseek(struct file *file, loff_t offset, int whence)
  				m->read_pos = offset;
  				retval = file->f_pos = offset;
  			}
-		}
+		} else
+			file->f_pos = offset;
  	}
  	file->f_version = m->version;
  	mutex_unlock(&m->lock);

This does not appear to be a problem, in linux man page, the behaver seems clearly defined:

DESCRIPTION
pread() reads up to count bytes from file descriptor fd at
offset off-set (from the start of the file) into the buffer starting at buf. The file offset is not changed.
pwrite() writes up to count bytes from the buffer starting at
buf to the file descriptor fd at offset offset. The file
offset is not changed.