| Message ID | 1622176367-12608-1-git-send-email-vidya.srinivas@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [i-g-t,RFC] tests/drm_read: Fix subtest invalid-buffer | expand |
On Fri, May 28, 2021 at 12:41 AM Vidya Srinivas <vidya.srinivas@intel.com> wrote: > > Using (void *)-1 directly in read is aborting on chrome systems. > Following message is seen. > > Starting subtest: invalid-buffer > *** buffer overflow detected ***: terminated > Received signal SIGABRT. > Stack trace: > Aborted (core dumped) > > Patch just adds a pointer variable and uses it in read. > Tested on ChromeOS on TGL (Delbin) and JSL (Drawlat) Tested-by: Mark Yacoub <markyacoub@chromium.org> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > tests/drm_read.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/drm_read.c b/tests/drm_read.c > index ccf9d822fd8d..2fdec5be4078 100644 > --- a/tests/drm_read.c > +++ b/tests/drm_read.c > @@ -103,10 +103,11 @@ static void teardown(int fd) > static void test_invalid_buffer(int in) > { > int fd = setup(in, 0); > + void *add = (void *)-1; > > alarm(1); > > - igt_assert_eq(read(fd, (void *)-1, 4096), -1); > + igt_assert_eq(read(fd, add, 4096), -1); > igt_assert_eq(errno, EFAULT); > > teardown(fd); > -- > 2.7.4 >
Thank you very much Mark, for testing the patch and providing the "Tested-by" tag. Regards Vidya -----Original Message----- From: Mark Yacoub <markyacoub@chromium.org> Sent: Saturday, June 5, 2021 12:13 AM To: Srinivas, Vidya <vidya.srinivas@intel.com> Cc: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org; Almahallawy, Khaled <khaled.almahallawy@intel.com>; Lin, Charlton <charlton.lin@intel.com> Subject: Re: [PATCH i-g-t] [RFC] tests/drm_read: Fix subtest invalid-buffer On Fri, May 28, 2021 at 12:41 AM Vidya Srinivas <vidya.srinivas@intel.com> wrote: > > Using (void *)-1 directly in read is aborting on chrome systems. > Following message is seen. > > Starting subtest: invalid-buffer > *** buffer overflow detected ***: terminated Received signal SIGABRT. > Stack trace: > Aborted (core dumped) > > Patch just adds a pointer variable and uses it in read. > Tested on ChromeOS on TGL (Delbin) and JSL (Drawlat) Tested-by: Mark Yacoub <markyacoub@chromium.org> > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > tests/drm_read.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/drm_read.c b/tests/drm_read.c index > ccf9d822fd8d..2fdec5be4078 100644 > --- a/tests/drm_read.c > +++ b/tests/drm_read.c > @@ -103,10 +103,11 @@ static void teardown(int fd) static void > test_invalid_buffer(int in) { > int fd = setup(in, 0); > + void *add = (void *)-1; > > alarm(1); > > - igt_assert_eq(read(fd, (void *)-1, 4096), -1); > + igt_assert_eq(read(fd, add, 4096), -1); > igt_assert_eq(errno, EFAULT); > > teardown(fd); > -- > 2.7.4 >
On Fri, May 28, 2021 at 10:02:47AM +0530, Vidya Srinivas wrote: > Using (void *)-1 directly in read is aborting on chrome systems. > Following message is seen. > > Starting subtest: invalid-buffer > *** buffer overflow detected ***: terminated > Received signal SIGABRT. > Stack trace: > Aborted (core dumped) > > Patch just adds a pointer variable and uses it in read. > > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > tests/drm_read.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/drm_read.c b/tests/drm_read.c > index ccf9d822fd8d..2fdec5be4078 100644 > --- a/tests/drm_read.c > +++ b/tests/drm_read.c > @@ -103,10 +103,11 @@ static void teardown(int fd) > static void test_invalid_buffer(int in) > { > int fd = setup(in, 0); > + void *add = (void *)-1; > > alarm(1); > > - igt_assert_eq(read(fd, (void *)-1, 4096), -1); > + igt_assert_eq(read(fd, add, 4096), -1); > igt_assert_eq(errno, EFAULT); > > teardown(fd); This looked weird but then I checked what glibc is actually doing. This is FORTIFY_SOURCE in action, and read() checks the buffer with __builtin_object_size() that it has room for the read. Which it can only do here if the address is a literal. Reviewed-by: Petri Latvala <petri.latvala@intel.com>
Thank you so much Petri. Regards Vidya -----Original Message----- From: Latvala, Petri <petri.latvala@intel.com> Sent: Monday, June 21, 2021 3:59 PM To: Srinivas, Vidya <vidya.srinivas@intel.com> Cc: intel-gfx@lists.freedesktop.org; igt-dev@lists.freedesktop.org; markyacoub@chromium.org; Lin, Charlton <charlton.lin@intel.com> Subject: Re: [Intel-gfx] [PATCH i-g-t] [RFC] tests/drm_read: Fix subtest invalid-buffer On Fri, May 28, 2021 at 10:02:47AM +0530, Vidya Srinivas wrote: > Using (void *)-1 directly in read is aborting on chrome systems. > Following message is seen. > > Starting subtest: invalid-buffer > *** buffer overflow detected ***: terminated Received signal SIGABRT. > Stack trace: > Aborted (core dumped) > > Patch just adds a pointer variable and uses it in read. > > Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> > --- > tests/drm_read.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tests/drm_read.c b/tests/drm_read.c index > ccf9d822fd8d..2fdec5be4078 100644 > --- a/tests/drm_read.c > +++ b/tests/drm_read.c > @@ -103,10 +103,11 @@ static void teardown(int fd) static void > test_invalid_buffer(int in) { > int fd = setup(in, 0); > + void *add = (void *)-1; > > alarm(1); > > - igt_assert_eq(read(fd, (void *)-1, 4096), -1); > + igt_assert_eq(read(fd, add, 4096), -1); > igt_assert_eq(errno, EFAULT); > > teardown(fd); This looked weird but then I checked what glibc is actually doing. This is FORTIFY_SOURCE in action, and read() checks the buffer with __builtin_object_size() that it has room for the read. Which it can only do here if the address is a literal. Reviewed-by: Petri Latvala <petri.latvala@intel.com>
diff --git a/tests/drm_read.c b/tests/drm_read.c index ccf9d822fd8d..2fdec5be4078 100644 --- a/tests/drm_read.c +++ b/tests/drm_read.c @@ -103,10 +103,11 @@ static void teardown(int fd) static void test_invalid_buffer(int in) { int fd = setup(in, 0); + void *add = (void *)-1; alarm(1); - igt_assert_eq(read(fd, (void *)-1, 4096), -1); + igt_assert_eq(read(fd, add, 4096), -1); igt_assert_eq(errno, EFAULT); teardown(fd);
Using (void *)-1 directly in read is aborting on chrome systems. Following message is seen. Starting subtest: invalid-buffer *** buffer overflow detected ***: terminated Received signal SIGABRT. Stack trace: Aborted (core dumped) Patch just adds a pointer variable and uses it in read. Signed-off-by: Vidya Srinivas <vidya.srinivas@intel.com> --- tests/drm_read.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)