Message ID | 20160920113759.23a1ee06@fiorina |
---|---|
State | New |
Headers | show |
On 20/09/2016 11:37, Tomáš Golembiovský wrote: > When --offset is set the apparent device size has to be adjusted > accordingly. Otherwise client may request read/write beyond the file end > which would fail. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > qemu-nbd.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 99297a5..629bce1 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -901,6 +901,13 @@ int main(int argc, char **argv) > exit(EXIT_FAILURE); > } > > + if (dev_offset >= fd_size) { > + error_report("Offset (%lu) has to be smaller than the image size (%lu)", > + dev_offset, fd_size); > + exit(EXIT_FAILURE); > + } > + fd_size -= dev_offset; > + > if (partition != -1) { > ret = find_partition(blk, partition, &dev_offset, &fd_size); > if (ret < 0) { > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
On 09/20/2016 04:37 AM, Tomáš Golembiovský wrote: [meta-comment]: Your series came through without any threading (you sent three threads, instead of patch 1 and 2 being marked In-Reply-To the 0/2 cover letter). > When --offset is set the apparent device size has to be adjusted > accordingly. Otherwise client may request read/write beyond the file end > which would fail. > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > --- > qemu-nbd.c | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 99297a5..629bce1 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -901,6 +901,13 @@ int main(int argc, char **argv) > exit(EXIT_FAILURE); > } Additional context: off_t dev_offset = 0; off_t fd_size; > > + if (dev_offset >= fd_size) { > + error_report("Offset (%lu) has to be smaller than the image size (%lu)", > + dev_offset, fd_size); Whoops, this fails to compile on 32-bit platforms. %lu is not necessarily synonymous with off_t values.
Whops, somehow I completely forgot about this. On Tue, 20 Sep 2016 09:09:59 -0500 Eric Blake <eblake@redhat.com> wrote: > On 09/20/2016 04:37 AM, Tomáš Golembiovský wrote: > > [meta-comment]: Your series came through without any threading (you sent > three threads, instead of patch 1 and 2 being marked In-Reply-To the 0/2 > cover letter). Thanks for the comment. Unfortunately it was my email client interfering. It should be better next time. > > When --offset is set the apparent device size has to be adjusted > > accordingly. Otherwise client may request read/write beyond the file end > > which would fail. > > > > Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> > > --- > > qemu-nbd.c | 7 +++++++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/qemu-nbd.c b/qemu-nbd.c > > index 99297a5..629bce1 100644 > > --- a/qemu-nbd.c > > +++ b/qemu-nbd.c > > @@ -901,6 +901,13 @@ int main(int argc, char **argv) > > exit(EXIT_FAILURE); > > } > > Additional context: > > off_t dev_offset = 0; > > off_t fd_size; > > > > > + if (dev_offset >= fd_size) { > > + error_report("Offset (%lu) has to be smaller than the image size (%lu)", > > + dev_offset, fd_size); > > Whoops, this fails to compile on 32-bit platforms. %lu is not > necessarily synonymous with off_t values. After some digging I figured off_t is in fact signed type. That makes the formatting wrong everywhere. Unfortunately I didn't find any good definition of the type. Any suggestion what format flag should I use? Or should I just use a temporary variable of known size for that? Thanks, Tomas
On 10/03/2016 08:50 AM, Tomáš Golembiovský wrote: >> Additional context: >> >> off_t dev_offset = 0; >> >> off_t fd_size; >> >>> >>> + if (dev_offset >= fd_size) { >>> + error_report("Offset (%lu) has to be smaller than the image size (%lu)", >>> + dev_offset, fd_size); >> >> Whoops, this fails to compile on 32-bit platforms. %lu is not >> necessarily synonymous with off_t values. > > After some digging I figured off_t is in fact signed type. That makes > the formatting wrong everywhere. Unfortunately I didn't find any good > definition of the type. Any suggestion what format flag should I use? Or > should I just use a temporary variable of known size for that? Easiest is probably casting to a type with an easier format flag, as in either of: error_report("offset %lld ...", (long long) dev_offset) error_report("offset %jd ...", (intmax_t) dev_offset) off_t is particularly problematic because there is no magic % sequence reserved for it, nothing in <inttypes.h> for it, and there are 32-bit compilation environments where it is still 32 bits (although qemu prefers to explicitly request large-file compilation so that off_t is always 64 bits)
diff --git a/qemu-nbd.c b/qemu-nbd.c index 99297a5..629bce1 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -901,6 +901,13 @@ int main(int argc, char **argv) exit(EXIT_FAILURE); } + if (dev_offset >= fd_size) { + error_report("Offset (%lu) has to be smaller than the image size (%lu)", + dev_offset, fd_size); + exit(EXIT_FAILURE); + } + fd_size -= dev_offset; + if (partition != -1) { ret = find_partition(blk, partition, &dev_offset, &fd_size); if (ret < 0) {
When --offset is set the apparent device size has to be adjusted accordingly. Otherwise client may request read/write beyond the file end which would fail. Signed-off-by: Tomáš Golembiovský <tgolembi@redhat.com> --- qemu-nbd.c | 7 +++++++ 1 file changed, 7 insertions(+)