Getting rid of size_t in IPC

[John Abd-El-Malek]

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.  This leads to the interesting scenario of 32bit and 64bit processes talking using our IPC channel.  While the communication only uses one or two messages now, there will be problems if any size_t's are sent across, since Visual Studio's crt typedefs it as unsigned int/__int64 depending on the platform.  Also, if the past is anything to go by, once we have the capability to talk between 32 and 64 bit processes, it will probably get used in other places.

So it seems we have no choice but to switch from size_t back to int for IPC.  Granted it's not as clean, but it's needed.  I'll be sending out code reviews, but I wanted to give a heads up so people know why it's happening

[Chris Evans]

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.  This leads to the interesting scenario of 32bit and 64bit processes talking using our IPC channel.  While the communication only uses one or two messages now, there will be problems if any size_t's are sent across, since Visual Studio's crt typedefs it as unsigned int/__int64 depending on the platform.  Also, if the past is anything to go by, once we have the capability to talk between 32 and 64 bit processes, it will probably get used in other places.

So it seems we have no choice but to switch from size_t back to int for IPC.  Granted it's not as clean, but it's needed.  I'll be sending out code reviews, but I wanted to give a heads up so people know why it's happening

Are you talking about all IPC, or just IPC between NaCl processes and the browser? size_t is unsigned whereas int is not. Perhaps "unsigned int" or "uint32_t" is preferable to avoid accidentally introducing security problems?

We'd also need to watch out for integer truncation issues on 64-bit.

Cheers

Chris

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Peter Kasting]

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.

Can we fix that?

This leads to the interesting scenario of 32bit and 64bit processes talking using our IPC channel.  While the communication only uses one or two messages now, there will be problems if any size_t's are sent across, since Visual Studio's crt typedefs it as unsigned int/__int64 depending on the platform.

size_t isn't the only type that can vary.  How about e.g. ptrdiff_t.

PK

[John Abd-El-Malek]

Yep, to everyone who emailed me off the list as well, I should have been more clear: we'd want to use something unsigned like int32_t.

On Wed, Feb 3, 2010 at 6:09 PM, Chris Evans <cev...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.  This leads to the interesting scenario of 32bit and 64bit processes talking using our IPC channel.  While the communication only uses one or two messages now, there will be problems if any size_t's are sent across, since Visual Studio's crt typedefs it as unsigned int/__int64 depending on the platform.  Also, if the past is anything to go by, once we have the capability to talk between 32 and 64 bit processes, it will probably get used in other places.

So it seems we have no choice but to switch from size_t back to int for IPC.  Granted it's not as clean, but it's needed.  I'll be sending out code reviews, but I wanted to give a heads up so people know why it's happening

Are you talking about all IPC, or just IPC between NaCl processes and the browser?

Since the same serialization code is used everywhere, this would have to be for all IPC.

[John Abd-El-Malek]

On Wed, Feb 3, 2010 at 6:10 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.

Can we fix that?

No, this is a limitation caused by the OS.  The 32 bit sandbox that NaCl doesn't work on 64 bit systems, so NaCl modules will have to run inside a 64 bit process there.

This leads to the interesting scenario of 32bit and 64bit processes talking using our IPC channel.  While the communication only uses one or two messages now, there will be problems if any size_t's are sent across, since Visual Studio's crt typedefs it as unsigned int/__int64 depending on the platform.

size_t isn't the only type that can vary.  How about e.g. ptrdiff_t.

We don't send ptrdiff_t between processes, but either way, we shouldn't be sending anything having to do with pointers between processes.  There have been a number of security bug fixes related to sending pointers.

 

PK

[Peter Kasting]

On Wed, Feb 3, 2010 at 6:17 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 6:10 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.

Can we fix that?

No, this is a limitation caused by the OS.  The 32 bit sandbox that NaCl doesn't work on 64 bit systems, so NaCl modules will have to run inside a 64 bit process there.

That wasn't a grammatical sentence, and sadly as a result I'm not quite sure what you meant.

My recollection was that NaCl was building a different sandbox for 64-bit systems.  Is it still a 32 bit process that runs in WoW or something?

I'm just really uneasy about this change.  size_t -> uint32_t feels icky anyway, but besides that lots of the other types can vary too.  Nothing prevents a compiler from making int or long 64 bits.  Maybe we don't have any environments where this is true now, but I wouldn't necessarily rely on that being true forever, with Chrome getting ported to more places.  If you want the types to be safe between any process, you should consider switching the IPC layer to only use explicitly sized types.

PK

[Mark Mentovai]

We'll wind up with similar problems on the Mac when we run a 64-bit
browser but need to host a 32-bit plugin.

I think we should avoid non-precise-sized types in IPC.

Mark

[John Abd-El-Malek]

On Wed, Feb 3, 2010 at 6:25 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 6:17 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 6:10 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.

Can we fix that?

No, this is a limitation caused by the OS.  The 32 bit sandbox that NaCl doesn't work on 64 bit systems, so NaCl modules will have to run inside a 64 bit process there.

That wasn't a grammatical sentence, and sadly as a result I'm not quite sure what you meant.

... that NaCl _uses_ doesn't ...

My recollection was that NaCl was building a different sandbox for 64-bit systems.  Is it still a 32 bit process that runs in WoW or something?

It's a 64 bit process.  Chrome will still be 32 bit even on 64 bit Windows. 

I'm just really uneasy about this change.  size_t -> uint32_t feels icky anyway, but besides that lots of the other types can vary too.  Nothing prevents a compiler from making int or long 64 bits.  Maybe we don't have any environments where this is true now, but I wouldn't necessarily rely on that being true forever, with Chrome getting ported to more places.  If you want the types to be safe between any process, you should consider switching the IPC layer to only use explicitly sized types.

It doesn't matter if on this theoretical platform ints are always 64 bits.  The problem is when we have two binaries on one OS that need to talk to each other and one uses 32 bit ints and the other 64 bit ints.

PK

[John Abd-El-Malek]

On Wed, Feb 3, 2010 at 6:30 PM, Mark Mentovai <ma...@chromium.org> wrote:

We'll wind up with similar problems on the Mac when we run a 64-bit
browser but need to host a 32-bit plugin.

Interesting, I wasn't aware that Chrome on Mac will be 64 bit?

[Peter Kasting]

On Wed, Feb 3, 2010 at 6:43 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 6:25 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 6:17 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 6:10 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.

No, this is a limitation caused by the OS.  The 32 bit sandbox that NaCl doesn't work on 64 bit systems, so NaCl modules will have to run inside a 64 bit process there.

... that NaCl _uses_ doesn't ...

It's a 64 bit process.  Chrome will still be 32 bit even on 64 bit Windows. 

So, backwards from what you originally said?  I'm still confused, but I guess it doesn't matter.

Also, I can pretty much guarantee you Chrome won't be 32-bit only on Windows forever.

I agree with Mark; just used explicitly-sized types everywhere, it's clear and safe and if someone adds a new message it's obvious what to do.

PK

[John Abd-El-Malek]

On Wed, Feb 3, 2010 at 6:48 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 6:43 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 6:25 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 6:17 PM, John Abd-El-Malek <j...@chromium.org> wrote:

On Wed, Feb 3, 2010 at 6:10 PM, Peter Kasting <pkas...@google.com> wrote:

On Wed, Feb 3, 2010 at 5:53 PM, John Abd-El-Malek <j...@chromium.org> wrote:

NaCl uses a 32bit process for sandboxing, even on 64 bit Windows.

No, this is a limitation caused by the OS.  The 32 bit sandbox that NaCl doesn't work on 64 bit systems, so NaCl modules will have to run inside a 64 bit process there.

... that NaCl _uses_ doesn't ...

It's a 64 bit process.  Chrome will still be 32 bit even on 64 bit Windows. 

So, backwards from what you originally said?  I'm still confused, but I guess it doesn't matter.

Yeah, sorry for the confusion, my first email was backwards.

 

Also, I can pretty much guarantee you Chrome won't be 32-bit only on Windows forever.

Agreed, but that's not the motivation for this change.

 

I agree with Mark; just used explicitly-sized types everywhere, it's clear and safe and if someone adds a new message it's obvious what to do.

Agreed, which is why this is happening :) 

PK

[Peter Kasting]

On Wed, Feb 3, 2010 at 6:51 PM, John Abd-El-Malek <j...@chromium.org> wrote:

I agree with Mark; just used explicitly-sized types everywhere, it's clear and safe and if someone adds a new message it's obvious what to do.

Agreed, which is why this is happening :)

OK, I thought you were only changing size_t, whereas I was suggesting changing all non-sized args.

PK 

[John Abd-El-Malek]

Gregory and I have been doing this (otherwise there's no point in fixing size_t's :) ).  I figured only size_t will be controversial so I thought I'd send an email to explain.

PK 

[Peter Kasting]

Cool.  Thanks for the heads-up, I'm glad for the explanation.

PK 

[Mark Mentovai]

John Abd-El-Malek wrote:
> Interesting, I wasn't aware that Chrome on Mac will be 64 bit?

Yes, it definitely will be. We're just waiting on support from a
couple of dependencies now - mostly Breakpad.

Mark