* [PATCH] commands: mem: truncate mem device size to fit the loff_t file size @ 2018-11-06 16:14 Lucas Stach 2018-11-07 7:47 ` Sascha Hauer 2018-11-07 13:45 ` Peter Mamonov 0 siblings, 2 replies; 5+ messages in thread From: Lucas Stach @ 2018-11-06 16:14 UTC (permalink / raw) To: barebox On 64bit arches the file covering the whole address space is larger than what can be represented in the loff_t type (s64) used for the file size. Thus the size of this device is interpreted as negative in a lot of places. Fix this by truncating the size to fit the file size type. Signed-off-by: Lucas Stach <l.stach@pengutronix.de> --- commands/mem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/commands/mem.c b/commands/mem.c index eb91ade05a88..cdd7a492d0d5 100644 --- a/commands/mem.c +++ b/commands/mem.c @@ -96,7 +96,8 @@ static int mem_probe(struct device_d *dev) dev->priv = cdev; cdev->name = (char*)dev->resource[0].name; - cdev->size = (unsigned long)resource_size(&dev->resource[0]); + cdev->size = min(resource_size(&dev->resource[0]), + (unsigned long long)S64_MAX); cdev->ops = &memops; cdev->dev = dev; -- 2.19.1 _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] commands: mem: truncate mem device size to fit the loff_t file size 2018-11-06 16:14 [PATCH] commands: mem: truncate mem device size to fit the loff_t file size Lucas Stach @ 2018-11-07 7:47 ` Sascha Hauer 2018-11-07 13:45 ` Peter Mamonov 1 sibling, 0 replies; 5+ messages in thread From: Sascha Hauer @ 2018-11-07 7:47 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox On Tue, Nov 06, 2018 at 05:14:07PM +0100, Lucas Stach wrote: > On 64bit arches the file covering the whole address space is larger than > what can be represented in the loff_t type (s64) used for the file size. > Thus the size of this device is interpreted as negative in a lot of > places. Fix this by truncating the size to fit the file size type. > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > commands/mem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Applied, thanks Sascha > > diff --git a/commands/mem.c b/commands/mem.c > index eb91ade05a88..cdd7a492d0d5 100644 > --- a/commands/mem.c > +++ b/commands/mem.c > @@ -96,7 +96,8 @@ static int mem_probe(struct device_d *dev) > dev->priv = cdev; > > cdev->name = (char*)dev->resource[0].name; > - cdev->size = (unsigned long)resource_size(&dev->resource[0]); > + cdev->size = min(resource_size(&dev->resource[0]), > + (unsigned long long)S64_MAX); > cdev->ops = &memops; > cdev->dev = dev; > > -- > 2.19.1 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox > -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 | _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] commands: mem: truncate mem device size to fit the loff_t file size 2018-11-06 16:14 [PATCH] commands: mem: truncate mem device size to fit the loff_t file size Lucas Stach 2018-11-07 7:47 ` Sascha Hauer @ 2018-11-07 13:45 ` Peter Mamonov 2018-11-07 16:08 ` Lucas Stach 1 sibling, 1 reply; 5+ messages in thread From: Peter Mamonov @ 2018-11-07 13:45 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox Hi, Lucas, On Tue, Nov 06, 2018 at 05:14:07PM +0100, Lucas Stach wrote: > On 64bit arches the file covering the whole address space is larger than > what can be represented in the loff_t type (s64) used for the file size. > Thus the size of this device is interpreted as negative in a lot of > places. Fix this by truncating the size to fit the file size type. While this is probably an appropriate solution for some architectures, it will cause problems on MIPS64. The root of evil is MIPS64 address space segmentation: https://docplayer.net/docs-images/25/4553906/images/21-0.png). Most notably your patch prevents access to the xkphys segment, where most peripheral devices of a SoC are accessible. Regards, Peter > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de> > --- > commands/mem.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/commands/mem.c b/commands/mem.c > index eb91ade05a88..cdd7a492d0d5 100644 > --- a/commands/mem.c > +++ b/commands/mem.c > @@ -96,7 +96,8 @@ static int mem_probe(struct device_d *dev) > dev->priv = cdev; > > cdev->name = (char*)dev->resource[0].name; > - cdev->size = (unsigned long)resource_size(&dev->resource[0]); > + cdev->size = min(resource_size(&dev->resource[0]), > + (unsigned long long)S64_MAX); > cdev->ops = &memops; > cdev->dev = dev; > > -- > 2.19.1 > > > _______________________________________________ > barebox mailing list > barebox@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/barebox _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] commands: mem: truncate mem device size to fit the loff_t file size 2018-11-07 13:45 ` Peter Mamonov @ 2018-11-07 16:08 ` Lucas Stach 2018-11-07 17:07 ` Peter Mamonov 0 siblings, 1 reply; 5+ messages in thread From: Lucas Stach @ 2018-11-07 16:08 UTC (permalink / raw) To: Peter Mamonov; +Cc: barebox Am Mittwoch, den 07.11.2018, 16:45 +0300 schrieb Peter Mamonov: > Hi, Lucas, > > On Tue, Nov 06, 2018 at 05:14:07PM +0100, Lucas Stach wrote: > > On 64bit arches the file covering the whole address space is larger than > > what can be represented in the loff_t type (s64) used for the file size. > > Thus the size of this device is interpreted as negative in a lot of > > places. Fix this by truncating the size to fit the file size type. > > While this is probably an appropriate solution for some architectures, it will > cause problems on MIPS64. The root of evil is MIPS64 address space > segmentation: https://docplayer.net/docs-images/25/4553906/images/21-0.png). > Most notably your patch prevents access to the xkphys segment, where most > peripheral devices of a SoC are accessible. Isn't this a problem for MIPS64 even before my patch? For example the "mw" command uses open_and_lseek on the dev/mem file with the address of the write being the seek offset. As this is a loff_t value, seeking to something with the highest order bit set won't do what you expect to happen, right? Regards, Lucas _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] commands: mem: truncate mem device size to fit the loff_t file size 2018-11-07 16:08 ` Lucas Stach @ 2018-11-07 17:07 ` Peter Mamonov 0 siblings, 0 replies; 5+ messages in thread From: Peter Mamonov @ 2018-11-07 17:07 UTC (permalink / raw) To: Lucas Stach; +Cc: barebox On Wed, Nov 07, 2018 at 05:08:17PM +0100, Lucas Stach wrote: > Am Mittwoch, den 07.11.2018, 16:45 +0300 schrieb Peter Mamonov: > > Hi, Lucas, > > > > On Tue, Nov 06, 2018 at 05:14:07PM +0100, Lucas Stach wrote: > > > On 64bit arches the file covering the whole address space is larger than > > > what can be represented in the loff_t type (s64) used for the file size. > > > Thus the size of this device is interpreted as negative in a lot of > > > places. Fix this by truncating the size to fit the file size type. > > > > While this is probably an appropriate solution for some architectures, it will > > cause problems on MIPS64. The root of evil is MIPS64 address space > > segmentation: https://docplayer.net/docs-images/25/4553906/images/21-0.png). > > Most notably your patch prevents access to the xkphys segment, where most > > peripheral devices of a SoC are accessible. > > Isn't this a problem for MIPS64 even before my patch? For example the > "mw" command uses open_and_lseek on the dev/mem file with the address > of the write being the seek offset. As this is a loff_t value, seeking > to something with the highest order bit set won't do what you expect to > happen, right? You are right. I fixed this in a straightforward manner (see the patch below). Guess it's time to find a better solution :) Author: Peter Mamonov <pmamonov@gmail.com> Date: Thu Jul 6 13:32:53 2017 +0300 FIXME: fs: fix memory access for 64bit MIPS diff --git a/fs/fs.c b/fs/fs.c index f61ee091b5..45190812c5 100644 --- a/fs/fs.c +++ b/fs/fs.c @@ -941,8 +941,6 @@ loff_t lseek(int fildes, loff_t offset, int whence) case SEEK_SET: if (f->size != FILE_SIZE_STREAM && offset > f->size) goto out; - if (offset < 0) - goto out; pos = offset; break; case SEEK_CUR: @@ -960,10 +958,6 @@ loff_t lseek(int fildes, loff_t offset, int whence) } pos = fsdrv->lseek(&f->fsdev->dev, f, pos); - if (pos < 0) { - errno = -pos; - return -1; - } return pos; Regards, Peter > > Regards, > Lucas _______________________________________________ barebox mailing list barebox@lists.infradead.org http://lists.infradead.org/mailman/listinfo/barebox ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-07 17:07 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-06 16:14 [PATCH] commands: mem: truncate mem device size to fit the loff_t file size Lucas Stach 2018-11-07 7:47 ` Sascha Hauer 2018-11-07 13:45 ` Peter Mamonov 2018-11-07 16:08 ` Lucas Stach 2018-11-07 17:07 ` Peter Mamonov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox