Linux-EROFS Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] erofs-utils: fix fragmentoff overflow for large packed inode
@ 2022-12-13  7:16 Yue Hu
  2022-12-13  8:27 ` Gao Xiang
  0 siblings, 1 reply; 3+ messages in thread
From: Yue Hu @ 2022-12-13  7:16 UTC (permalink / raw)
  To: linux-erofs; +Cc: Yue Hu, zhangwen

From: Yue Hu <huyue2@coolpad.com>

The return value of ftell() is a long int, use ftello{64} instead. Also,
use lseek64 for large file position to avoid this error if we have it.
And need to return at once if they fail.

Signed-off-by: Yue Hu <huyue2@coolpad.com>
---
 configure.ac    |  2 ++
 lib/fragments.c | 34 +++++++++++++++++++++++++++++-----
 lib/inode.c     |  9 +++++++--
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/configure.ac b/configure.ac
index a736ff0..5c9666a 100644
--- a/configure.ac
+++ b/configure.ac
@@ -190,6 +190,8 @@ AC_CHECK_FUNCS(m4_flatten([
 	llistxattr
 	memset
 	realpath
+	lseek64
+	ftello64
 	pread64
 	pwrite64
 	strdup
diff --git a/lib/fragments.c b/lib/fragments.c
index e69ae47..4302cd5 100644
--- a/lib/fragments.c
+++ b/lib/fragments.c
@@ -3,7 +3,18 @@
  * Copyright (C), 2022, Coolpad Group Limited.
  * Created by Yue Hu <huyue2@coolpad.com>
  */
+#ifndef _LARGEFILE_SOURCE
+#define _LARGEFILE_SOURCE
+#endif
+#ifndef _LARGEFILE64_SOURCE
+#define _LARGEFILE64_SOURCE
+#endif
+#ifndef _FILE_OFFSET_BITS
+#define _FILE_OFFSET_BITS 64
+#endif
+#ifndef _GNU_SOURCE
 #define _GNU_SOURCE
+#endif
 #include <stdlib.h>
 #include <unistd.h>
 #include "erofs/err.h"
@@ -30,6 +41,13 @@ static struct list_head dupli_frags[FRAGMENT_HASHSIZE];
 static FILE *packedfile;
 const char *frags_packedname = "packed_file";
 
+#ifndef HAVE_LSEEK64
+static inline off_t lseek64(int fd, u64 offset, int set)
+{
+	return lseek(fd, offset, set);
+}
+#endif
+
 static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
 					 u32 crc)
 {
@@ -53,8 +71,7 @@ static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
 	if (!data)
 		return -ENOMEM;
 
-	ret = lseek(fd, inode->i_size - length, SEEK_SET);
-	if (ret < 0) {
+	if (lseek64(fd, inode->i_size - length, SEEK_SET) < 0) {
 		ret = -errno;
 		goto out;
 	}
@@ -113,8 +130,7 @@ int z_erofs_fragments_dedupe(struct erofs_inode *inode, int fd, u32 *tofcrc)
 	if (inode->i_size <= EROFS_TOF_HASHLEN)
 		return 0;
 
-	ret = lseek(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET);
-	if (ret < 0)
+	if (lseek64(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET) < 0)
 		return -errno;
 
 	ret = read(fd, data_to_hash, EROFS_TOF_HASHLEN);
@@ -192,9 +208,17 @@ void z_erofs_fragments_commit(struct erofs_inode *inode)
 int z_erofs_pack_fragments(struct erofs_inode *inode, void *data,
 			   unsigned int len, u32 tofcrc)
 {
+#ifdef HAVE_FTELLO64
+	off64_t offset = ftello64(packedfile);
+#else
+	off_t offset = ftello(packedfile);
+#endif
 	int ret;
 
-	inode->fragmentoff = ftell(packedfile);
+	if (offset < 0)
+		return -errno;
+
+	inode->fragmentoff = (erofs_off_t)offset;
 	inode->fragment_size = len;
 
 	if (fwrite(data, len, 1, packedfile) != 1)
diff --git a/lib/inode.c b/lib/inode.c
index 9de4dec..a8510f3 100644
--- a/lib/inode.c
+++ b/lib/inode.c
@@ -1196,7 +1196,10 @@ struct erofs_inode *erofs_mkfs_build_special_from_fd(int fd, const char *name)
 	struct erofs_inode *inode;
 	int ret;
 
-	lseek(fd, 0, SEEK_SET);
+	ret = lseek(fd, 0, SEEK_SET);
+	if (ret < 0)
+		return ERR_PTR(-errno);
+
 	ret = fstat64(fd, &st);
 	if (ret)
 		return ERR_PTR(-errno);
@@ -1223,7 +1226,9 @@ struct erofs_inode *erofs_mkfs_build_special_from_fd(int fd, const char *name)
 
 	ret = erofs_write_compressed_file(inode, fd);
 	if (ret == -ENOSPC) {
-		lseek(fd, 0, SEEK_SET);
+		ret = lseek(fd, 0, SEEK_SET);
+		if (ret < 0)
+			return ERR_PTR(-errno);
 		ret = write_uncompressed_file_from_fd(inode, fd);
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] erofs-utils: fix fragmentoff overflow for large packed inode
  2022-12-13  7:16 [PATCH] erofs-utils: fix fragmentoff overflow for large packed inode Yue Hu
@ 2022-12-13  8:27 ` Gao Xiang
  2022-12-13  9:20   ` Yue Hu
  0 siblings, 1 reply; 3+ messages in thread
From: Gao Xiang @ 2022-12-13  8:27 UTC (permalink / raw)
  To: Yue Hu; +Cc: Yue Hu, linux-erofs, Khem Raj, zhangwen

(cc Khem Raj <raj.khem@gmail.com>... Since I'd like to apply this first
 and then enable _FILE_OFFSET_BITS=64 unconditionally so that we could
 clean up all hard-coded _FILE_OFFSET_BITS in the source code.)

Hi Yue,

On Tue, Dec 13, 2022 at 03:16:43PM +0800, Yue Hu wrote:
> From: Yue Hu <huyue2@coolpad.com>
> 
> The return value of ftell() is a long int, use ftello{64} instead. Also,
> use lseek64 for large file position to avoid this error if we have it.
> And need to return at once if they fail.
> 
> Signed-off-by: Yue Hu <huyue2@coolpad.com>
> ---
>  configure.ac    |  2 ++
>  lib/fragments.c | 34 +++++++++++++++++++++++++++++-----
>  lib/inode.c     |  9 +++++++--
>  3 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/configure.ac b/configure.ac
> index a736ff0..5c9666a 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -190,6 +190,8 @@ AC_CHECK_FUNCS(m4_flatten([
>  	llistxattr
>  	memset
>  	realpath
> +	lseek64
> +	ftello64
>  	pread64
>  	pwrite64
>  	strdup
> diff --git a/lib/fragments.c b/lib/fragments.c
> index e69ae47..4302cd5 100644
> --- a/lib/fragments.c
> +++ b/lib/fragments.c
> @@ -3,7 +3,18 @@
>   * Copyright (C), 2022, Coolpad Group Limited.
>   * Created by Yue Hu <huyue2@coolpad.com>
>   */
> +#ifndef _LARGEFILE_SOURCE
> +#define _LARGEFILE_SOURCE
> +#endif
> +#ifndef _LARGEFILE64_SOURCE
> +#define _LARGEFILE64_SOURCE
> +#endif
> +#ifndef _FILE_OFFSET_BITS
> +#define _FILE_OFFSET_BITS 64
> +#endif
> +#ifndef _GNU_SOURCE
>  #define _GNU_SOURCE
> +#endif
>  #include <stdlib.h>
>  #include <unistd.h>
>  #include "erofs/err.h"
> @@ -30,6 +41,13 @@ static struct list_head dupli_frags[FRAGMENT_HASHSIZE];
>  static FILE *packedfile;
>  const char *frags_packedname = "packed_file";
>  
> +#ifndef HAVE_LSEEK64
> +static inline off_t lseek64(int fd, u64 offset, int set)
> +{
> +	return lseek(fd, offset, set);
> +}
> +#endif

Could we use another function name other than native lseek64?

> +
>  static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
>  					 u32 crc)
>  {
> @@ -53,8 +71,7 @@ static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
>  	if (!data)
>  		return -ENOMEM;
>  
> -	ret = lseek(fd, inode->i_size - length, SEEK_SET);
> -	if (ret < 0) {
> +	if (lseek64(fd, inode->i_size - length, SEEK_SET) < 0) {
>  		ret = -errno;
>  		goto out;
>  	}
> @@ -113,8 +130,7 @@ int z_erofs_fragments_dedupe(struct erofs_inode *inode, int fd, u32 *tofcrc)
>  	if (inode->i_size <= EROFS_TOF_HASHLEN)
>  		return 0;
>  
> -	ret = lseek(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET);
> -	if (ret < 0)
> +	if (lseek64(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET) < 0)
>  		return -errno;
>  
>  	ret = read(fd, data_to_hash, EROFS_TOF_HASHLEN);
> @@ -192,9 +208,17 @@ void z_erofs_fragments_commit(struct erofs_inode *inode)
>  int z_erofs_pack_fragments(struct erofs_inode *inode, void *data,
>  			   unsigned int len, u32 tofcrc)
>  {
> +#ifdef HAVE_FTELLO64
> +	off64_t offset = ftello64(packedfile);
> +#else
> +	off_t offset = ftello(packedfile);
> +#endif
>  	int ret;
>  
> -	inode->fragmentoff = ftell(packedfile);
> +	if (offset < 0)
> +		return -errno;
> +
> +	inode->fragmentoff = (erofs_off_t)offset;
>  	inode->fragment_size = len;
>  
>  	if (fwrite(data, len, 1, packedfile) != 1)
> diff --git a/lib/inode.c b/lib/inode.c
> index 9de4dec..a8510f3 100644
> --- a/lib/inode.c
> +++ b/lib/inode.c
> @@ -1196,7 +1196,10 @@ struct erofs_inode *erofs_mkfs_build_special_from_fd(int fd, const char *name)
>  	struct erofs_inode *inode;
>  	int ret;
>  
> -	lseek(fd, 0, SEEK_SET);
> +	ret = lseek(fd, 0, SEEK_SET);
> +	if (ret < 0)
> +		return ERR_PTR(-errno);

This should be in a seperate patch, please don't fix it here.

> +
>  	ret = fstat64(fd, &st);
>  	if (ret)
>  		return ERR_PTR(-errno);
> @@ -1223,7 +1226,9 @@ struct erofs_inode *erofs_mkfs_build_special_from_fd(int fd, const char *name)
>  
>  	ret = erofs_write_compressed_file(inode, fd);
>  	if (ret == -ENOSPC) {
> -		lseek(fd, 0, SEEK_SET);
> +		ret = lseek(fd, 0, SEEK_SET);
> +		if (ret < 0)

Same here.

Thanks,
Gao Xiang

> +			return ERR_PTR(-errno);
>  		ret = write_uncompressed_file_from_fd(inode, fd);
>  	}
>  
> -- 
> 2.17.1

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] erofs-utils: fix fragmentoff overflow for large packed inode
  2022-12-13  8:27 ` Gao Xiang
@ 2022-12-13  9:20   ` Yue Hu
  0 siblings, 0 replies; 3+ messages in thread
From: Yue Hu @ 2022-12-13  9:20 UTC (permalink / raw)
  To: Gao Xiang; +Cc: Yue Hu, linux-erofs, Khem Raj, zhangwen

On Tue, 13 Dec 2022 16:27:37 +0800
Gao Xiang <hsiangkao@linux.alibaba.com> wrote:

> (cc Khem Raj <raj.khem@gmail.com>... Since I'd like to apply this first
>  and then enable _FILE_OFFSET_BITS=64 unconditionally so that we could
>  clean up all hard-coded _FILE_OFFSET_BITS in the source code.)
> 
> Hi Yue,
> 
> On Tue, Dec 13, 2022 at 03:16:43PM +0800, Yue Hu wrote:
> > From: Yue Hu <huyue2@coolpad.com>
> > 
> > The return value of ftell() is a long int, use ftello{64} instead. Also,
> > use lseek64 for large file position to avoid this error if we have it.
> > And need to return at once if they fail.
> > 
> > Signed-off-by: Yue Hu <huyue2@coolpad.com>
> > ---
> >  configure.ac    |  2 ++
> >  lib/fragments.c | 34 +++++++++++++++++++++++++++++-----
> >  lib/inode.c     |  9 +++++++--
> >  3 files changed, 38 insertions(+), 7 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index a736ff0..5c9666a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -190,6 +190,8 @@ AC_CHECK_FUNCS(m4_flatten([
> >  	llistxattr
> >  	memset
> >  	realpath
> > +	lseek64
> > +	ftello64
> >  	pread64
> >  	pwrite64
> >  	strdup
> > diff --git a/lib/fragments.c b/lib/fragments.c
> > index e69ae47..4302cd5 100644
> > --- a/lib/fragments.c
> > +++ b/lib/fragments.c
> > @@ -3,7 +3,18 @@
> >   * Copyright (C), 2022, Coolpad Group Limited.
> >   * Created by Yue Hu <huyue2@coolpad.com>
> >   */
> > +#ifndef _LARGEFILE_SOURCE
> > +#define _LARGEFILE_SOURCE
> > +#endif
> > +#ifndef _LARGEFILE64_SOURCE
> > +#define _LARGEFILE64_SOURCE
> > +#endif
> > +#ifndef _FILE_OFFSET_BITS
> > +#define _FILE_OFFSET_BITS 64
> > +#endif
> > +#ifndef _GNU_SOURCE
> >  #define _GNU_SOURCE
> > +#endif
> >  #include <stdlib.h>
> >  #include <unistd.h>
> >  #include "erofs/err.h"
> > @@ -30,6 +41,13 @@ static struct list_head dupli_frags[FRAGMENT_HASHSIZE];
> >  static FILE *packedfile;
> >  const char *frags_packedname = "packed_file";
> >  
> > +#ifndef HAVE_LSEEK64
> > +static inline off_t lseek64(int fd, u64 offset, int set)
> > +{
> > +	return lseek(fd, offset, set);
> > +}
> > +#endif  
> 
> Could we use another function name other than native lseek64?
> 
> > +
> >  static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
> >  					 u32 crc)
> >  {
> > @@ -53,8 +71,7 @@ static int z_erofs_fragments_dedupe_find(struct erofs_inode *inode, int fd,
> >  	if (!data)
> >  		return -ENOMEM;
> >  
> > -	ret = lseek(fd, inode->i_size - length, SEEK_SET);
> > -	if (ret < 0) {
> > +	if (lseek64(fd, inode->i_size - length, SEEK_SET) < 0) {
> >  		ret = -errno;
> >  		goto out;
> >  	}
> > @@ -113,8 +130,7 @@ int z_erofs_fragments_dedupe(struct erofs_inode *inode, int fd, u32 *tofcrc)
> >  	if (inode->i_size <= EROFS_TOF_HASHLEN)
> >  		return 0;
> >  
> > -	ret = lseek(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET);
> > -	if (ret < 0)
> > +	if (lseek64(fd, inode->i_size - EROFS_TOF_HASHLEN, SEEK_SET) < 0)
> >  		return -errno;
> >  
> >  	ret = read(fd, data_to_hash, EROFS_TOF_HASHLEN);
> > @@ -192,9 +208,17 @@ void z_erofs_fragments_commit(struct erofs_inode *inode)
> >  int z_erofs_pack_fragments(struct erofs_inode *inode, void *data,
> >  			   unsigned int len, u32 tofcrc)
> >  {
> > +#ifdef HAVE_FTELLO64
> > +	off64_t offset = ftello64(packedfile);
> > +#else
> > +	off_t offset = ftello(packedfile);
> > +#endif
> >  	int ret;
> >  
> > -	inode->fragmentoff = ftell(packedfile);
> > +	if (offset < 0)
> > +		return -errno;
> > +
> > +	inode->fragmentoff = (erofs_off_t)offset;
> >  	inode->fragment_size = len;
> >  
> >  	if (fwrite(data, len, 1, packedfile) != 1)
> > diff --git a/lib/inode.c b/lib/inode.c
> > index 9de4dec..a8510f3 100644
> > --- a/lib/inode.c
> > +++ b/lib/inode.c
> > @@ -1196,7 +1196,10 @@ struct erofs_inode *erofs_mkfs_build_special_from_fd(int fd, const char *name)
> >  	struct erofs_inode *inode;
> >  	int ret;
> >  
> > -	lseek(fd, 0, SEEK_SET);
> > +	ret = lseek(fd, 0, SEEK_SET);
> > +	if (ret < 0)
> > +		return ERR_PTR(-errno);  
> 
> This should be in a seperate patch, please don't fix it here.
> 
> > +
> >  	ret = fstat64(fd, &st);
> >  	if (ret)
> >  		return ERR_PTR(-errno);
> > @@ -1223,7 +1226,9 @@ struct erofs_inode *erofs_mkfs_build_special_from_fd(int fd, const char *name)
> >  
> >  	ret = erofs_write_compressed_file(inode, fd);
> >  	if (ret == -ENOSPC) {
> > -		lseek(fd, 0, SEEK_SET);
> > +		ret = lseek(fd, 0, SEEK_SET);
> > +		if (ret < 0)  
> 
> Same here.

Okay, will update for all in v2.

> 
> Thanks,
> Gao Xiang
> 
> > +			return ERR_PTR(-errno);
> >  		ret = write_uncompressed_file_from_fd(inode, fd);
> >  	}
> >  
> > -- 
> > 2.17.1  


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-12-13  9:15 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-13  7:16 [PATCH] erofs-utils: fix fragmentoff overflow for large packed inode Yue Hu
2022-12-13  8:27 ` Gao Xiang
2022-12-13  9:20   ` Yue Hu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox