[Vm-dev] Fwd: Merging FilesAttributesPlugin
Alistair Grant
akgrant0710 at gmail.com
Thu Jan 4 15:06:13 UTC 2018
Hi Eliot,
Yet another lesson (for me) on why I shouldn't rush emails out just
before going to bed.
The crash is due to #pointerFrom: only checking for a byte type
object, so sending a random string of the correct length will cause
the VM to crash. I've tightened the parameter checking, and neither
the 32 bit or 64 bit images crash now.
Updated version:
Name: FileAttributesPlugin.oscog-AlistairGrant.25
Author: AlistairGrant
Time: 4 January 2018, 4:02:11.405148 pm
UUID: 5b76e8bb-69e1-4db4-b529-9de1249e63e0
Ancestors: FileAttributesPlugin.oscog-eem.24
Version 1.2.3:
- Ensure that #pointerFrom: is given a ByteArray, not just a byte
object, which can be a string.
Thanks,
Alistair
On 4 January 2018 at 00:05, Alistair Grant <akgrant0710 at gmail.com> wrote:
> Hi Eliot,
>
> I'm able to reliably crash the 64 bit VM by passing a string to
> primitiveClosedir in the FileAttributesPlugin (which should never be
> done in practice, I've got it as a test for the parameter checking):
>
>
> primitiveClosedir
> "Close the directory stream for dirPointerOop. Answer
> dirPointerOop on success.
> Raise PrimErrBadArgument if the parameter is not a ByteArray
> length size(void *).
> If closedir() returns an error raise PrimitiveOSError."
>
> | dirPointerOop dirStream result |
> <export: true>
> <var: 'dirStream' type: 'osdir *'>
> dirPointerOop := interpreterProxy stackValue: 0.
>
>
> ">>>> Putting the following back prevents the crash..."
> (interpreterProxy is: dirPointerOop KindOf: 'ByteArray') ifFalse:
> [^interpreterProxy primitiveFailFor: PrimErrBadArgument].
> "<<<<"
>
> dirStream := self pointerFrom: dirPointerOop.
> dirStream ifNil:
> [^interpreterProxy primitiveFailFor: PrimErrBadArgument].
> result := self closedir: dirStream dp.
> result = 0 ifFalse:
> [^interpreterProxy primitiveFailForOSError: self unableToCloseDir].
> self free: dirStream.
> interpreterProxy pop: 2 thenPush: dirPointerOop
>
>
> I removed the explicit is:KindOf: ByteArray check as you said that the
> checks in pointerFrom: would be sufficient.
>
> Would you mind letting me know what your preferred solution is (put
> the explicit ByteArray check back in, or look to fix the checks in
> pointerFrom:, or ...)?
>
> Just for reference, pointerFrom: is:
>
>
> pointerFrom: directoryPointerBytes
> "Answer the machine address contained in anExternalAddressOop."
>
> | ptr addressUnion idx |
> <returnTypeC: 'void *'>
> <var: 'ptr' type: 'unsigned char *'>
> <var: 'addressUnion' type: 'union {void *address; unsigned char
> bytes[sizeof(void *)];}'>
> ((interpreterProxy isBytes: directoryPointerBytes) and:
> [(interpreterProxy stSizeOf: directoryPointerBytes) = self
> sizeOfPointer])
> ifFalse: [^ nil].
> ptr := interpreterProxy arrayValueOf: directoryPointerBytes.
> idx := 0.
> [idx < self sizeOfPointer] whileTrue:
> [self cCode: 'addressUnion.bytes[idx] = ptr[idx]'.
> idx := idx + 1].
> ^ self cCode: 'addressUnion.address' inSmalltalk: [addressUnion]
>
>
> Thanks,
> Alistair
>
>
>
> On 3 January 2018 at 20:45, Alistair Grant <akgrant0710 at gmail.com> wrote:
>> Hi Eliot,
>>
>> On 3 January 2018 at 20:20, Alistair Grant <akgrant0710 at gmail.com> wrote:
>>> Hi Eliot,
>>>
>>> On 3 January 2018 at 19:04, Eliot Miranda <eliot.miranda at gmail.com> wrote:
>>>>
>>>> Hi Alistair,
>>>>
>>>> On Tue, Jan 2, 2018 at 2:42 AM, Alistair Grant <akgrant0710 at gmail.com> wrote:
>>>>>
>>>>>
>>>>> Hi Jakob,
>>>>>
>>>>> On 2 January 2018 at 06:46, Jakob Reschke <forums.jakob at resfarm.de> wrote:
>>>>> >
>>>>> >
>>>>> > Am 02.01.2018 04:43 schrieb "Eliot Miranda" <eliot.miranda at gmail.com>:
>>>>> >
>>>>> > ... inst vars such as sCOFfn are set up in initialiseModule. You're querying the SecurityPlugin on every access as in
>>>>> >
>>>>> >
>>>>> > canStatFilePath: aPathCString length: length
>>>>> > ...
>>>>> > (hasSecurityPlugin = 0) ifTrue: [^ true].
>>>>> > sCOFfn := interpreterProxy ioLoadFunction: 'secCanOpenFileOfSizeWritable' From: 'SecurityPlugin'.
>>>>> > sCOFfn ~= 0
>>>>> > ...
>>>>> >
>>>>> >
>>>>> > I have never looked at this before, so: can this variable name be spelled out, please, so it becomes legible for the uninitiated folk?
>>>>>
>>>>> I've made this change in my image, so it will flow through on the next update.
>>>>
>>>>
>>>> I don't think it's necessary. The long vars are unwieldy and I'm sure Jakob understands the convention now.
>>>
>>> OK.
>>>
>>>
>>>> I just did a minor edit on the plain. I wonder if it's time to move the package to the VMMaker repository. In any case I'll start including it in builds.
>>>
>>> Yes, please :-)
>>>
>>> My build system has suddenly decided that it can't find
>>> libpulse-simple. I can't see that any of the changes you've made will
>>> impact it, but I'll work on getting my build working again and running
>>> it through the test suite.
>>
>> Deleting the running docker container and going back to the clean
>> image fixed the libpulse-simple problem.
>>
>> And all the automated tests pass (FileAttributesPlugin.oscog-eem.22,
>> Ubuntu 16.04).
>>
>> Thanks!
>> Alistair
More information about the Vm-dev
mailing list