1610017 - Generated calls to IPC Recv methods that pass Tainted data

Status: RESOLVED FIXED

(Core :: IPC, enhancement)

Description

[Tom Ritter [:tjr]]

After Bug 1610007, this is for actually doing something with the tainted keyword.

Comment 1

[Tom Ritter [:tjr]]

Attachment: Bug 1610017 - Actually taint data when tainted is specified for IPC methods r?nika

Comment 2

[Tom Ritter [:tjr]]

Attachment: Codegen change on a ipl in VIRTUAL_CALL_CLASSES

Comment 3

[Tom Ritter [:tjr]]

Attachment: Codegen change on regular a ipdl

Comment 4

[Nika Layzell [:nika] (ooo, ni? for response)]

re: https://bugzilla.mozilla.org/attachment.cgi?id=9122448&action=diff, could you also include the corresponding .cpp changes? I don't think the signature you declared there will be compatible with the generated code which you're passing in.

re: https://bugzilla.mozilla.org/attachment.cgi?id=9122450&action=diff, is this the code you are wanting to create? It seems surprising to me that you'd want to move each argument into a Tainted temporary as you're passing them into the method.

Giving feedback on the exact code will be easier with the actual code for the Tainted template, as right now I don't know how it behaves with certain types, such as references, used as a type parameter.

Comment 5

[Tom Ritter [:tjr]]

(In reply to :Nika Layzell (ni? for response) from comment #4)

re: https://bugzilla.mozilla.org/attachment.cgi?id=9122448&action=diff, could you also include the corresponding .cpp changes? I don't think the signature you declared there will be compatible with the generated code which you're passing in.

Yes, I think you're right. I will fiddle some more. (My environment just broke so I have to fix it first.)

re: https://bugzilla.mozilla.org/attachment.cgi?id=9122450&action=diff, is this the code you are wanting to create? It seems surprising to me that you'd want to move each argument into a Tainted temporary as you're passing them into the method.

I am less familiar with the nuances of stuff like std::move and std::forward and rrvalues. I thought it was, but if it surprised you then it probably isn't. If we taint stuff I can omit a move (if it was going to be present) if that's the correct course of action.

Giving feedback on the exact code will be easier with the actual code for the Tainted template, as right now I don't know how it behaves with certain types, such as references, used as a type parameter.

Bug 1610039 has the template

Comment 6

[Tom Ritter [:tjr]]

Attachment: Codegen change on a ipl in VIRTUAL_CALL_CLASSES (Part 2)

Attached is the codegen change on the cpp file, as well as the edits I made to the in-tree subclass.

Note that this code doesn't work. The .cpp call to RecvProgressChange is passing a long; while the RecvProgressChange declaration expects a const long&. The type of the argument being passed is known to the codegen as a long, so we generate a Tainted<long> which cannot be implicitly cast to a Tainted<const long&>.

I'm not sure what the correct solution to this is, but it seems related to the move into the temporary, so I'm going to await feedback.

Comment 7

[Tom Ritter [:tjr]]

Attachment: Codegen change on a regular ipdl

Comment 8

[Tom Ritter [:tjr]]

Attachment: Codegen change on a ipl in VIRTUAL_CALL_CLASSES

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6850a80bf162
Actually taint data when tainted is specified for IPC methods r=nika

Comment 10

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/6850a80bf162