<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 4, 2018 at 5:08 AM, Dilger, Andreas <span dir="ltr"><<a href="mailto:andreas.dilger@intel.com" target="_blank">andreas.dilger@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On May 3, 2018, at 22:19, Wenwen Wang <<a href="mailto:wang6495@umn.edu">wang6495@umn.edu</a>> wrote:<br>
> <br>
> On Tue, May 1, 2018 at 3:46 AM, Dan Carpenter <<a href="mailto:dan.carpenter@oracle.com">dan.carpenter@oracle.com</a>> wrote:<br>
>> On Mon, Apr 30, 2018 at 05:56:10PM -0500, Wenwen Wang wrote:<br>
>>> However, given that the user data resides in the user space, a malicious<br>
>>> user-space process can race to change the data between the two copies. By<br>
>>> doing so, the attacker can provide a data with an inconsistent version,<br>
>>> e.g., v1 version + v3 data. This can lead to logical errors in the<br>
>>> following execution in ll_dir_setstripe(), which performs different actions<br>
>>> according to the version specified by the field lmm_magic.<br>
>> <br>
>> This part is misleading.  The fix is to improve readability and make<br>
>> static checkers happy.  You're over dramatizing it to make people think<br>
>> it has a security impact when it doesn't.<br>
>> <br>
>> If the user wants to specify v1 data they can just say that on the first<br>
>> read.  They don't need to do funny tricks and race between the two<br>
>> reads.  It's allowed.<br>
>> <br>
>> In other words this allows the user to do something in a very<br>
>> complicated way which they are already allowed to do in a very simple<br>
>> straight forward way.<br>
>> <br>
>> regards,<br>
>> dan carpenter<br>
> <br>
> Thanks for your comment, Dan! How about this:<br>
> <br>
> However, given that the user data resides in the user space, a<br>
> malicious user-space process can race to change the data between the<br>
> two copies. By doing so, the attacker can provide a data with an<br>
> inconsistent version, e.g., v1 version + v3 data. The current kernel<br>
> can handle such inconsistent data. But, it may pose a potential<br>
> security risk for future implementations. Also, to improve code<br>
> readability and make static analysis tools happy, which will warn<br>
> about read-verify-re-read type bugs, this issue should be fixed.<br>
<br>
</div></div>There is nothing preventing the user from using struct lov_mds_md_v3 but<br>
filling in lmm_magic = LOV_MAGIC_V1 from the beginning, no need for a race.<br></blockquote><div><br></div><div>Right. No need for users to race. There might be a type confusion issue though if V1</div><div>object is  used as V3 or the other way. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
Cheers, Andreas<br>
--<br>
Andreas Dilger<br>
Lustre Principal Architect<br>
Intel Corporation<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div><span style="font-size:12.8px">Kangjie Lu</span><br></div><div>Assistant Professor</div><div>Department of Computer Science and Engineering</div><div>University of Minnesota</div><div><a href="mailto:kjlu@umn.edu" target="_blank">kjlu@umn.edu</a></div><div><a href="http://kangjie.lu" target="_blank">http://kangjie.lu</a></div></div></div></div></div>
</div></div>