Skip to content
Permalink
Browse files
Fix unsafe memory load/store from the argument encoder/decoder affect…
…ing ARM

https://bugs.webkit.org/show_bug.cgi?id=125674

Patch by Benjamin Poulain <bpoulain@apple.com> on 2013-12-12
Reviewed by Darin Adler.

Depending on the CPU and CPU config, load and store may have to be aligned.
The argument buffer has no particular alignment which can cause problems.

In this case, on ARMv7, strd and ldrd can have alignment restriction on 16 bytes.
The code encoding double and 64 bits integers was causing bugs.

To avoid problems, the encoders/decoders are modified to just use memcpy. The compiler optimizes
it away for the right instructions (clang uses two ldr/str in the case of 64bits values on ARMv7).

* Platform/CoreIPC/ArgumentDecoder.cpp:
(CoreIPC::decodeValueFromBuffer):
(CoreIPC::ArgumentDecoder::decode):
* Platform/CoreIPC/ArgumentEncoder.cpp:
(CoreIPC::copyValueToBuffer):
(CoreIPC::ArgumentEncoder::encode):


Canonical link: https://commits.webkit.org/143699@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@160529 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
Benjamin Poulain authored and BenjaminPoulain committed Dec 13, 2013
1 parent b2728b4 commit aada9c47636462fafcf1926fd8ef96004eadba2b
Showing 3 changed files with 57 additions and 39 deletions.
@@ -1,3 +1,26 @@
2013-12-12 Benjamin Poulain <bpoulain@apple.com>

Fix unsafe memory load/store from the argument encoder/decoder affecting ARM
https://bugs.webkit.org/show_bug.cgi?id=125674

Reviewed by Darin Adler.

Depending on the CPU and CPU config, load and store may have to be aligned.
The argument buffer has no particular alignment which can cause problems.

In this case, on ARMv7, strd and ldrd can have alignment restriction on 16 bytes.
The code encoding double and 64 bits integers was causing bugs.

To avoid problems, the encoders/decoders are modified to just use memcpy. The compiler optimizes
it away for the right instructions (clang uses two ldr/str in the case of 64bits values on ARMv7).

* Platform/CoreIPC/ArgumentDecoder.cpp:
(CoreIPC::decodeValueFromBuffer):
(CoreIPC::ArgumentDecoder::decode):
* Platform/CoreIPC/ArgumentEncoder.cpp:
(CoreIPC::copyValueToBuffer):
(CoreIPC::ArgumentEncoder::encode):

2013-12-12 Tim Horton <timothy_horton@apple.com>

Build fix for 32-bit.
@@ -126,13 +126,19 @@ bool ArgumentDecoder::decodeVariableLengthByteArray(DataReference& dataReference
return true;
}

template<typename Type>
static void decodeValueFromBuffer(Type& value, uint8_t*& bufferPosition)
{
memcpy(&value, bufferPosition, sizeof(value));
bufferPosition += sizeof(Type);
}

bool ArgumentDecoder::decode(bool& result)
{
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast<bool*>(m_bufferPos);
m_bufferPos += sizeof(result);
decodeValueFromBuffer(result, m_bufferPos);
return true;
}

@@ -141,8 +147,7 @@ bool ArgumentDecoder::decode(uint8_t& result)
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast<uint8_t*>(m_bufferPos);
m_bufferPos += sizeof(result);
decodeValueFromBuffer(result, m_bufferPos);
return true;
}

@@ -151,18 +156,16 @@ bool ArgumentDecoder::decode(uint16_t& result)
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<uint16_t*>(m_bufferPos);
m_bufferPos += sizeof(result);
decodeValueFromBuffer(result, m_bufferPos);
return true;
}

bool ArgumentDecoder::decode(uint32_t& result)
{
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<uint32_t*>(m_bufferPos);
m_bufferPos += sizeof(result);

decodeValueFromBuffer(result, m_bufferPos);
return true;
}

@@ -171,8 +174,7 @@ bool ArgumentDecoder::decode(uint64_t& result)
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<uint64_t*>(m_bufferPos);
m_bufferPos += sizeof(result);
decodeValueFromBuffer(result, m_bufferPos);
return true;
}

@@ -181,28 +183,25 @@ bool ArgumentDecoder::decode(int32_t& result)
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<uint32_t*>(m_bufferPos);
m_bufferPos += sizeof(result);
decodeValueFromBuffer(result, m_bufferPos);
return true;
}

bool ArgumentDecoder::decode(int64_t& result)
{
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<uint64_t*>(m_bufferPos);
m_bufferPos += sizeof(result);

decodeValueFromBuffer(result, m_bufferPos);
return true;
}

bool ArgumentDecoder::decode(float& result)
{
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<float*>(m_bufferPos);
m_bufferPos += sizeof(result);

decodeValueFromBuffer(result, m_bufferPos);
return true;
}

@@ -211,8 +210,7 @@ bool ArgumentDecoder::decode(double& result)
if (!alignBufferPosition(sizeof(result), sizeof(result)))
return false;

result = *reinterpret_cast_ptr<double*>(m_bufferPos);
m_bufferPos += sizeof(result);
decodeValueFromBuffer(result, m_bufferPos);
return true;
}

@@ -123,67 +123,64 @@ void ArgumentEncoder::encodeVariableLengthByteArray(const DataReference& dataRef
encodeFixedLengthData(dataReference.data(), dataReference.size(), 1);
}

template<typename Type>
static void copyValueToBuffer(Type value, uint8_t* bufferPosition)
{
memcpy(bufferPosition, &value, sizeof(Type));
}

void ArgumentEncoder::encode(bool n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast<bool*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(uint8_t n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast<uint8_t*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(uint16_t n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<uint16_t*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(uint32_t n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<uint32_t*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(uint64_t n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<uint64_t*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(int32_t n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<int32_t*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(int64_t n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<int64_t*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(float n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<float*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::encode(double n)
{
uint8_t* buffer = grow(sizeof(n), sizeof(n));

*reinterpret_cast_ptr<double*>(buffer) = n;
copyValueToBuffer(n, buffer);
}

void ArgumentEncoder::addAttachment(const Attachment& attachment)

0 comments on commit aada9c4

Please sign in to comment.