Re: [LTP] [PATCH] Fix FILECAPS test hanging for more than 12 hours

["Serge E. Hallyn" <serue@us.ibm.com>]

Quoting Garrett Cooper (yanegomi@gmail.com):
> On May 5, 2010, at 7:18 AM, Serge E. Hallyn wrote:
> 
> > Quoting Garrett Cooper (yanegomi@gmail.com):
> >>>                p = index(buf, '.')+1;
> > 
> > Jinkeys!  The intertubes archives insist I wrote that, but I'm finding
> > it hard to believe.
> > 
> >>> -               if (p==(char *)1) {
> >>> -                       tst_resm(TFAIL, "got a bad message from print_caps\n");
> >>> -                       tst_exit();
> >>> -               }
> >>> +               if (p==(char *)1)
> >>> +                       tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> >> 
> >>    This is a really incorrect way to do things. I think that the
> >> assumption made was that index(3) would return 0 ('\0') if it fails to
> >> find '.'. That's incorrect and would cause a segfault on some systems
> >> (does on FreeBSD at least... don't see why it would pass on Linux):
> >> 
> >> $ ~/test_null_inc
> >> Segmentation fault: 11 (core dumped)
> >> [garrcoop@bioshock ~]$ cat ~/test_null_inc.c
> >> #include <stdio.h>
> >> int
> >> main(void)
> >> {
> >> 	printf("%s\n", (NULL + 1));
> >> 	return 0;
> >> }
> > 
> > Well, that's different - you're dereferencing NULL+1, whereas I'm
> > just checking the the value of the pointer.  
> > 
> > Still what I did is darned ugly, cleanup below.
> > 
> > thanks,
> > -serge
> > 
> >>    Could you please change this to check and see whether or not index
> >> returns NULL instead of accessing memory like that?
> >>    Other than that, patch looks good.
> > 
> > From: Serge E. Hallyn <serue@us.ibm.com>
> > Date: Wed, 5 May 2010 02:59:05 -0500
> > Subject: [PATCH 1/1] check for index(3) returning NULL
> > 
> > Signed-off-by: Serge E. Hallyn <serue@us.ibm.com>
> > ---
> > .../kernel/security/filecaps/verify_caps_exec.c    |    5 +++--
> > 1 files changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/testcases/kernel/security/filecaps/verify_caps_exec.c b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > index c3f65a9..605f0f6 100644
> > --- a/testcases/kernel/security/filecaps/verify_caps_exec.c
> > +++ b/testcases/kernel/security/filecaps/verify_caps_exec.c
> > @@ -182,9 +182,10 @@ int fork_drop_and_exec(int keepperms, cap_t expected_caps)
> > 			tst_resm(TINFO, "got a bad seqno (c=%d, s=%d, seqno=%d)",
> > 				c, s, seqno);
> > 		}
> > -		p = index(buf, '.')+1;
> > -		if (p==(char *)1)
> > +		p = index(buf, '.');
> > +		if (!p)
> > 			tst_brkm(TFAIL, tst_exit, "got a bad message from print_caps\n");
> > +		p += 1;
> > 		actual_caps = cap_from_text(p);
> > 		if (cap_compare(actual_caps, expected_caps) != 0) {
> > 			capstxt = cap_to_text(expected_caps, NULL);
> 
> Looks good! If that's the complete diff, then Acked-by: Garrett Cooper <yanegomi@gmail.com>

Right - that one on top of the previous longer one, please.  (or I can rebase-squash
them and resend if Subrata prefers)

-serge

------------------------------------------------------------------------------
_______________________________________________
Ltp-list mailing list
Ltp-list@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/ltp-list