<html><body>
<p><tt>Hal Rosenstock <halr@voltaire.com> wrote on 11/04/2005 02:30:49 PM:<br>
<br>
> On Fri, 2005-11-04 at 17:06, Pradeep Satyanarayana wrote:<br>
> > I realize that address translation will be replaced shortly. However,<br>
> > here are a few things that<br>
> > I observed which I believe are important. <br>
> <br>
> Important to fix in what time frame ?<br>
> <br>
> > I recently saw an e-mail thread about compilation problems and <br>
> > data structure padding; this is in line with that.<br>
> > <br>
> > So that new incarnation does not face the same pitfalls of address<br>
> > translation, I will describe them here.<br>
> > <br>
> > When I tried running uatt it fails with -EFAULT. Debug revealed that<br>
> > it fails. The following <br>
> > copy_from_user() fails.<br>
> > <br>
> > ib_route = kmalloc(sizeof *ib_route, GFP_KERNEL);<br>
> > if (!ib_route) {<br>
> > result = -ENOMEM;<br>
> > goto err1;<br>
> > }<br>
> > <br>
> > if (copy_from_user(ib_route, cmd.ib_route, sizeof(ib_route))) {<br>
> > result = -EFAULT;<br>
> > goto err2;<br>
> > }<br>
> > <br>
> > In fact I believe this copy_from_user() is unnecessary since this will<br>
> > be actually filled in by "address translation" and <br>
> > passed back to user space later on.<br>
> <br>
> Not always. If I recall correctly, there is a case where this copy is<br>
> needed. It is not in the mode that uatt uses AT right now though.</tt><br>
<br>
<tt>Maybe true, but there is still a 32-bit app 64-bit kernel issue that needs to be</tt><br>
<tt>fixed, unless we agree to change the data structure to say incorporate a device_name</tt><br>
<tt>as you suggest below.</tt><br>
<br>
<tt><br>
> <br>
> > So, if I eliminate this copy_from_user(), uatt again fails with<br>
> > EFAULT in:<br>
> > <br>
> > if (copy_to_user((void __user *)(unsigned long)cmd.response,<br>
> > &resp, sizeof(resp))) {<br>
> > result = -EFAULT;<br>
> > goto err4;<br>
> > }<br>
> > <br>
> > The environment I was using a 32-bit app and 64-bit kernel on Power.<br>
> > The reason is <br>
> > struct ib_uat_route_by_ip_req has pointers in them (LP64 vs ILP32).<br>
> <br>
> This needs to be replaced by the port GID. Another alternative is the<br>
> name. This has been discussed before on the list.<br>
> <br>
> -- Hal<br>
> <br>
> > I am told a 64-bit app succeeded on a 64-bit kernel which confirmed my<br>
> > suspicions.<br>
> > <br>
> > Given that I took a quick look at all the places that copy_from_user()<br>
> > is used (I did not<br>
> > do this exercise for copy_to_user(), which would be the complete thing<br>
> > to do) and found <br>
> > that this (data structure size mismatch) potentially also occurs in<br>
> > user_mad,c. I did not see any anomalies</tt><br>
<br>
<tt>Even if we change struct ib_uat_route_by_ip_req, there still is user_mad.c that </tt><br>
<tt>needs to be looked into.</tt><br>
<br>
<br>
<tt>Pradeep</tt><br>
<tt>pradeep@us.ibm.com</tt><br>
<tt><br>
<br>
</tt></body></html>