[PATCH 1/3] tests: Demarshalling of very long array/string lengths.

[Michal Srb]

On pátek 17. srpna 2018 15:15:55 CEST Pekka Paalanen wrote:
> Hi Michal,

Hi,

Thank you for the reviews. I will work on the changes. Some answers below.

> > +		0xffffffff, 0xfffffffe, 0xfffffffd, 0xfffffffc,
> > +		0xfffff000, 0xffffd000, 0xffffc000, 0xffffb000
> 
> What is the rationale for the second row on values here?
> I can understand the first row which are very close to overflow.
> Why is 0xffffe000 not included?

The first line (0xffffffff, 0xfffffffe, 0xfffffffd, 0xfffffffc) is to trigger 
the overflow in DIV_ROUNDUP.

The second line (0xfffff000, 0xffffd000, 0xffffc000, 0xffffb000) are random 
lengths that are big enough to overflow the pointer `p` and wrap around to 
point it to some smaller address. We don't know what's in memory there, so 
trying different offsets hoping to hit address that is not mapped.

I'll add explanations for the values to the test.

The whole test is kinda heuristic, it is not guaranteed to cause crash. But 
there is good chance that it will. So if the code changes in future and some 
overflow is reintroduced, this has chance to discover it.

> The following are quite confusing to me:
> > +		msg[0] = 400200;
> 
> This is the sender id, which is irrelevant to the code being tested;
> can be arbitrary. Could use a comment /* sender id */.
> 
> > +		msg[1] = 24;
> 
> This might be due to bad examples already present in this file.
> 
> This is the ((size << 16) | opcode) field. The size is unused in the
> code to be tested, and the opcode is irrelevant too. However, you are
> repurposing this field for message size in a way that does not match
> the wire format. I find it confusing.
> 
> I also could not figure out where the 24 comes from. I would have
> expected:
> - header, 8 bytes
> - 's' argument length, 4 bytes
> - followed by the literal string data including the terminating NUL and
>   padding up to the next 4 byte boundary
> 
> Of course, the excercise is to lie about the string length, so the
> literal string data is irrelevant. The arbitrary 24 works here, but my
> complaint is that there is no recorded rationale where it came from.
> 
> This file already contains mistakes like line 420:
> 
> 	msg[1] = 12;		/* size = 12, opcode = 0 */
> 
> That's actually size=0, opcode=12. I guess that this mislead you.

Right, I just copied it from the other tests. I'll update and comment the 
values.

Michal