[Vm-dev] Fwd: Merging FilesAttributesPlugin

Alistair Grant akgrant0710 at gmail.com
Wed Jan 3 23:05:49 UTC 2018


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