[openib-general] Re: Fwd: [Bug 91] sizeof(srp_indirect_buf) wrongon 64-bit platforms

[Arne Redlich]

Ishai Rabinovitz <ishai at mellanox.co.il> writes:

> On Tue, May 23, 2006 at 10:10:05AM +0300, Arne Redlich wrote:
>> Roland Dreier <rdreier at cisco.com> writes:
>> 
>> >  > Roland, maybe this means we need scsi/srp.h in svn for now?
>> >  > svn is supposed to work on 2.6.16 ...
>> >
>> > As far as I can tell the bug in the header has no effect on how the IB
>> > SRP initiator works.
>> 
>> Roland,
>> 
>> I'm afraid it *does* have an effect, unfortunately. There's the following code in ib_srp.c::srp_map_data(), around the lines 540 - 550:
>> 
>>       struct srp_indirect_buf *buf = (void *) cmd->add_data;
>> 
>>       /* snip */
>> 
>>       buf->table_desc.va  = cpu_to_be64(req->cmd->dma +
>>                                         sizeof *cmd +
>>                                         sizeof *buf);
>> 
>> So if a target actually RDMA Reads the indirect descriptor table, it will use a wrong address.
>> 
>
> It looks to me that there is no effect after all.
> This buf->table_desc.va should point to the desc_list array in the srp_indirect_buf.
> When the code enters the values to this array (buf->desc_list[i]) it uses the 
> address that is corresponding to sizeof *buf.
>
> To sum it up, there will be a change in the address the target sees but the 
> data will still be in the address the target sees.

No, unfortunately this is wrong.
The code posted below resembles the offending part in ib_srp.c. It results in this output on an x86_64:

sizeof p: 24
offset of table_desc: 0
offset of len: 16
offset of desc_list: 20
addr. of p: ffff81003c389f28
p->table_desc.va: ffff81003c389f40
p->desc_list[0]:  ffff81003c389f3c

Arne
-- 
Arne Redlich
Xiranet Communications GmbH

/* --------------- cut here ------------------------------------------------ */

#include <linux/module.h>
#include <scsi/srp.h>

/* my <scsi/srp.h> is already fixed, here's the broken version again */
struct srp_indirect_buf_old {
        struct srp_direct_buf   table_desc;
	__be32                  len;
	struct srp_direct_buf   desc_list[0] __attribute__((packed));
};

MODULE_LICENSE("GPL");

static void __exit test_fini(void)
{
	return;
}

static int __init test_init(void)
{
	struct srp_indirect_buf_old buf, *p;

	p = &buf;

	p->table_desc.va  = (u64)(unsigned long)p;
	p->table_desc.va += sizeof(struct srp_indirect_buf_old);

	printk("sizeof p: %lu\n", sizeof(*p));
	printk("offset of table_desc: %lu\n",
	       offsetof(struct srp_indirect_buf_old, table_desc));
	printk("offset of len: %lu\n",
	       offsetof(struct srp_indirect_buf_old, len));
	printk("offset of desc_list: %lu\n",
	       offsetof(struct srp_indirect_buf_old, desc_list));
	printk("addr. of p: %p\n", p);
	printk("p->table_desc.va: %llx\n", p->table_desc.va);
	printk("p->desc_list[0]:  %p\n", &p->desc_list[0]);

        return 0;
}

module_init(test_init);
module_exit(test_fini);