Issue 164373: Fix seccomp sandbox for gcc44

Issue 164373: Fix seccomp sandbox for gcc44 (Closed)

Created:
11 years, 4 months ago by Joel Stanley (old)

Modified:
9 years, 7 months ago

Reviewers:
Dean McNamee, Markus (顧孟勤)

CC:
chromium-reviews_googlegroups.com

Visibility:
Public.

Description

Fix seccomp sandbox for gcc44 Constness of return values and paramaters were causing compiler errors. BUG=19120

Patch Set 1 #

Created: 11 years, 4 months ago

Download [raw] [tar.bz2]

		Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+2 lines, -2 lines)			Patch
	M	sandbox/linux/seccomp/debug.cc	View		1 chunk	+1 line, -1 line	0 comments	Download
	M	sandbox/linux/seccomp/sandbox.cc	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 8 (0 generated)

Expand Messages | Collapse Messages

Dean McNamee

Does this already use base? We should probably not be using atoi... On Tue, Aug ...

11 years, 4 months ago (2009-08-12 04:39:31 UTC) #2

Does this already use base?  We should probably not be using atoi...

On Tue, Aug 11, 2009 at 8:06 PM, <joel.stan@gmail.com> wrote:
> Reviewers: Markus (=E9=A1=A7=E5=AD=9F=E5=8B=A4), Dean McNamee,
>
> Description:
> Fix seccomp for gcc44
>
> Constness of return values and paramaters was causing compiler errors.
>
> BUG=3D19120
>
> Please review this at http://codereview.chromium.org/164373
>
> Affected files:
> =C2=A0M sandbox/linux/seccomp/debug.cc
> =C2=A0M sandbox/linux/seccomp/sandbox.cc
>
>
> Index: sandbox/linux/seccomp/debug.cc
> diff --git a/sandbox/linux/seccomp/debug.cc b/sandbox/linux/seccomp/debug=
.cc
> index
> b4f30a4fc96a2fa379aa70eb00270d327632d4ba..467b460fc857fb304e494a1ded1d88f=
09ae24174
> 100644
> --- a/sandbox/linux/seccomp/debug.cc
> +++ b/sandbox/linux/seccomp/debug.cc
> @@ -148,7 +148,7 @@ void Debug::syscall(int sysnum, const char* msg, int
> call) {
> =C2=A0 =C2=A0 }
> =C2=A0 =C2=A0 char unnamed[40] =3D "Unnamed syscall #";
> =C2=A0 =C2=A0 if (!sysname) {
> - =C2=A0 =C2=A0 =C2=A0itoa(strrchr(sysname =3D unnamed, '\000'), sysnum);
> + =C2=A0 =C2=A0 =C2=A0itoa(const_cast<char*>(strrchr(sysname =3D unnamed,=
 '\000')), sysnum);
> =C2=A0 =C2=A0 }
> =C2=A0 =C2=A0 #if defined(__NR_socketcall) || defined(__NR_ipc)
> =C2=A0 =C2=A0 char extra[40];
> Index: sandbox/linux/seccomp/sandbox.cc
> diff --git a/sandbox/linux/seccomp/sandbox.cc
> b/sandbox/linux/seccomp/sandbox.cc
> index
> 0c3e499a62cc8ba36f9cbf46cf576c60e091b35b..d6497a19d56e40cb256a2f1141d867d=
7ddd17fc8
> 100644
> --- a/sandbox/linux/seccomp/sandbox.cc
> +++ b/sandbox/linux/seccomp/sandbox.cc
> @@ -396,7 +396,7 @@ void Sandbox::startSandbox() {
> =C2=A0 =C2=A0 for (Maps::const_iterator iter =3D maps.begin(); iter !=3D =
maps.end();
> ++iter){
> =C2=A0 =C2=A0 =C2=A0 Library* library =3D *iter;
> =C2=A0 =C2=A0 =C2=A0 for (const char **ptr =3D libs; *ptr; ptr++) {
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0char *name =3D strstr(iter.name().c_str(), *=
ptr);
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0const char *name =3D strstr(iter.name().c_st=
r(), *ptr);
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (name) {
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 char ch =3D name[strlen(*ptr)];
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (ch < 'A' || (ch > 'Z' && ch < 'a')=
 || ch > 'z') {
>
>
>

Joel Stanley (old)

Can we land the patch to fix the build now, and fix the other issues ...

11 years, 4 months ago (2009-08-12 04:43:58 UTC) #3

Dean McNamee

Epic duplication of 2 line patches... Anyway, I'll let AGL or Markus deicde what they ...

11 years, 4 months ago (2009-08-12 04:51:14 UTC) #4

Markus (顧孟勤)

This code cannot make calls into any "complicated" glibc function. Anything that could take locks, ...

11 years, 4 months ago (2009-08-12 06:04:32 UTC) #5

This code cannot make calls into any "complicated" glibc function. Anything
that could take locks, that modifies errno, or that accesses global state i=
s
out of bounds.
Fortunately, itoa() is a function that we defined ourselves, and which we
know to be safe.


Markus

On Tue, Aug 11, 2009 at 21:38, Dean McNamee <deanm@chromium.org> wrote:

> Does this already use base?  We should probably not be using atoi...
>
> On Tue, Aug 11, 2009 at 8:06 PM, <joel.stan@gmail.com> wrote:
> > Reviewers: Markus (=E9=A1=A7=E5=AD=9F=E5=8B=A4), Dean McNamee,
> >
> > Description:
> > Fix seccomp for gcc44
> >
> > Constness of return values and paramaters was causing compiler errors.
> >
> > BUG=3D19120
> >
> > Please review this at http://codereview.chromium.org/164373
> >
> > Affected files:
> >  M sandbox/linux/seccomp/debug.cc
> >  M sandbox/linux/seccomp/sandbox.cc
> >
> >
> > Index: sandbox/linux/seccomp/debug.cc
> > diff --git a/sandbox/linux/seccomp/debug.cc
> b/sandbox/linux/seccomp/debug.cc
> > index
> >
> b4f30a4fc96a2fa379aa70eb00270d327632d4ba..467b460fc857fb304e494a1ded1d88f=
09ae24174
> > 100644
> > --- a/sandbox/linux/seccomp/debug.cc
> > +++ b/sandbox/linux/seccomp/debug.cc
> > @@ -148,7 +148,7 @@ void Debug::syscall(int sysnum, const char* msg, in=
t
> > call) {
> >     }
> >     char unnamed[40] =3D "Unnamed syscall #";
> >     if (!sysname) {
> > -      itoa(strrchr(sysname =3D unnamed, '\000'), sysnum);
> > +      itoa(const_cast<char*>(strrchr(sysname =3D unnamed, '\000')),
> sysnum);
> >     }
> >     #if defined(__NR_socketcall) || defined(__NR_ipc)
> >     char extra[40];
> > Index: sandbox/linux/seccomp/sandbox.cc
> > diff --git a/sandbox/linux/seccomp/sandbox.cc
> > b/sandbox/linux/seccomp/sandbox.cc
> > index
> >
> 0c3e499a62cc8ba36f9cbf46cf576c60e091b35b..d6497a19d56e40cb256a2f1141d867d=
7ddd17fc8
> > 100644
> > --- a/sandbox/linux/seccomp/sandbox.cc
> > +++ b/sandbox/linux/seccomp/sandbox.cc
> > @@ -396,7 +396,7 @@ void Sandbox::startSandbox() {
> >     for (Maps::const_iterator iter =3D maps.begin(); iter !=3D maps.end=
();
> > ++iter){
> >       Library* library =3D *iter;
> >       for (const char **ptr =3D libs; *ptr; ptr++) {
> > -        char *name =3D strstr(iter.name().c_str(), *ptr);
> > +        const char *name =3D strstr(iter.name().c_str(), *ptr);
> >         if (name) {
> >           char ch =3D name[strlen(*ptr)];
> >           if (ch < 'A' || (ch > 'Z' && ch < 'a') || ch > 'z') {
> >
> >
> >
>

Markus (顧孟勤)

Argh, doesn't gcc break POSIX by redefining strchr() and strstr()? Anyway, the changes look good. ...

11 years, 4 months ago (2009-08-12 06:06:19 UTC) #6

Argh, doesn't gcc break POSIX by redefining strchr() and strstr()? Anyway,
the changes look good.

Markus

On Tue, Aug 11, 2009 at 20:06, <joel.stan@gmail.com> wrote:

> Reviewers: Markus (=E9=A1=A7=E5=AD=9F=E5=8B=A4), Dean McNamee,
>
> Description:
> Fix seccomp for gcc44
>
> Constness of return values and paramaters was causing compiler errors.
>
> BUG=3D19120
>
> Please review this at http://codereview.chromium.org/164373
>
> Affected files:
>  M sandbox/linux/seccomp/debug.cc
>  M sandbox/linux/seccomp/sandbox.cc
>
>
> Index: sandbox/linux/seccomp/debug.cc
> diff --git a/sandbox/linux/seccomp/debug.cc
> b/sandbox/linux/seccomp/debug.cc
> index
> b4f30a4fc96a2fa379aa70eb00270d327632d4ba..467b460fc857fb304e494a1ded1d88f=
09ae24174
> 100644
> --- a/sandbox/linux/seccomp/debug.cc
> +++ b/sandbox/linux/seccomp/debug.cc
> @@ -148,7 +148,7 @@ void Debug::syscall(int sysnum, const char* msg, int
> call) {
>     }
>     char unnamed[40] =3D "Unnamed syscall #";
>     if (!sysname) {
> -      itoa(strrchr(sysname =3D unnamed, '\000'), sysnum);
> +      itoa(const_cast<char*>(strrchr(sysname =3D unnamed, '\000')), sysn=
um);
>     }
>     #if defined(__NR_socketcall) || defined(__NR_ipc)
>     char extra[40];
> Index: sandbox/linux/seccomp/sandbox.cc
> diff --git a/sandbox/linux/seccomp/sandbox.cc
> b/sandbox/linux/seccomp/sandbox.cc
> index
> 0c3e499a62cc8ba36f9cbf46cf576c60e091b35b..d6497a19d56e40cb256a2f1141d867d=
7ddd17fc8
> 100644
> --- a/sandbox/linux/seccomp/sandbox.cc
> +++ b/sandbox/linux/seccomp/sandbox.cc
> @@ -396,7 +396,7 @@ void Sandbox::startSandbox() {
>     for (Maps::const_iterator iter =3D maps.begin(); iter !=3D maps.end()=
;
> ++iter){
>       Library* library =3D *iter;
>       for (const char **ptr =3D libs; *ptr; ptr++) {
> -        char *name =3D strstr(iter.name().c_str(), *ptr);
> +        const char *name =3D strstr(iter.name().c_str(), *ptr);
>         if (name) {
>           char ch =3D name[strlen(*ptr)];
>           if (ch < 'A' || (ch > 'Z' && ch < 'a') || ch > 'z') {
>
>
>

Markus (顧孟勤)

Yes, please feel free to submit this change. Markus On Tue, Aug 11, 2009 at ...

11 years, 4 months ago (2009-08-12 06:07:19 UTC) #7

Dean McNamee

11 years, 4 months ago (2009-08-12 06:09:48 UTC) #8

Joel doesn't have commit.

Thanks

On Tue, Aug 11, 2009 at 11:06 PM, Markus Gutschke<markus@chromium.org> wrot=
e:
> Yes, please feel free to submit this change.
>
> Markus
>
> On Tue, Aug 11, 2009 at 21:43, <joel.stan@gmail.com> wrote:
>>
>> Can we land the patch to fix the build now, and fix the other issues
>> later? =A0Last time there was a gcc43/44 break that had a 2 line fix we
>> had 4 different people submit CL before a fix hit the tree. Epic
>> duplication of effort.
>>
>> http://codereview.chromium.org/164373
>
>

Expand Messages | Collapse Messages