[openib-general] [BUG] NULL pointer deref in ib_sa_mcmember_rec_callback()

[Grant Grundler]

On Mon, Mar 28, 2005 at 10:33:17PM -0800, Grant Grundler wrote:
> Same config/kernel as before: 2.6.11 + SVN gen2 version 2050.
...
> ip is at ib_sa_mcmember_rec_callback+0x90/0xe0 [ib_sa]
...
>  [<a0000002000a54d0>] ib_sa_mcmember_rec_callback+0x90/0xe0 [ib_sa]
>                                 sp=e000000101fcfd20 bsp=e000000101fc9020
>  [<a0000002000a5a30>] send_handler+0x110/0x280 [ib_sa]
>                                 sp=e000000101fcfd70 bsp=e000000101fc8fd0
>  [<a00000020011a5b0>] ib_mad_complete_send_wr+0x270/0x300 [ib_mad]
>                                 sp=e000000101fcfd70 bsp=e000000101fc8f90
...

Roland,
I can't unravel a64 asm as well as I'd like.
Fortunately, this ut the code is pretty straighforward.

+0x60 to +0x8c is the "true" part of this statement:

        if (mad) {
		struct ib_sa_mcmember_rec rec;

		ib_unpack(mcmember_rec_table, ARRAY_SIZE(mcmember_rec_table), mad->data, &rec);
		query->callback(status, &rec, query->context);
	} else
		query->callback(status, NULL, query->context);

And +0x90 is the "else" (false) part.


I *think*:
	r32 = *sa_query
	r33 = status
	r34 = *mad

Following asm is from sa_query.o - ie unlinked:

    101c:       04 07 fd 8c                         adds r35=-16,r32
    1020:       0a 40 00 44 09 39       [MMI]       cmp.eq p8,p9=0,r34;;
...
    105c:       40 00 00 43                   (p08) br.cond.dpnt.few 1090 <ib_sa_mcmember_rec_callback+0x90>
    1060:       11 00 00 00 01 00       [MIB]       nop.m 0x0
                        1062: PCREL21B  ib_unpack
    1066:       00 00 00 02 00 00                   nop.i 0x0
    106c:       08 00 00 50                         br.call.sptk.many b0=1060 <ib_sa_mcmember_rec_callback+0x60>;;

...
    1080:       0a 70 00 46 18 10       [MMI]       ld8 r14=[r35];;
    1086:       90 02 0c 30 20 00                   ld8 r41=[r3]
    108c:       00 00 04 00                         nop.i 0x0
==> 1090:       0a 40 20 1c 18 14       [MMI]       ld8 r8=[r14],8;; <==
    1096:       10 00 38 30 20 e0                   ld8 r1=[r14]
    109c:       80 08 00 07                         mov b7=r8
    10a0:       11 00 00 00 01 00       [MIB]       nop.m 0x0
    10a6:       00 00 00 02 00 00                   nop.i 0x0
(b) 10ac:       78 00 80 10                         br.call.sptk.many b0=b7;;
    10b0:       00 08 00 4c 00 21       [MII]       mov r1=r38
    10b6:       00 28 01 55 00 00                   mov.i ar.pfs=r37
    10bc:       40 0a 00 07                         mov b0=r36
    10c0:       11 60 40 19 00 21       [MIB]       adds r12=80,r12
    10c6:       00 00 00 02 00 80                   nop.i 0x0
    10cc:       08 00 84 00                         br.ret.sptk.many b0;;


My take is (b) is the indirect call with the differences in the parameters
factored out of the if/else statement.
+0xcc is the return from ib_sa_mcmember_rec_callback().

I'm pretty sure +0x90 trying to reference query->context.
tombstone showed r14 was zero.
So it would have blown up regardless if which path we took.
This is the first place query (r14) is dereferenced.

hth,
grant