On Fri, May 25, 2018 at 6:53 PM, Eric Sandeen <sandeen@xxxxxxxxxx> wrote:
> On 5/25/18 10:14 AM, Arnd Bergmann wrote:
>>
>> gcc-8 reports two warnings for the newly added getlabel/setlabel code:
>
>
> Thanks for catching these.
>
> The patch summary confuses me, what does "mark sb_fname as nonstring"
> mean in the context of the actual patch?
My mistake. I tried a few different approaches and ended up using the
subject line from an earlier version with a later patch.
The 'nonstring' annotation is a variable attribute that gets gcc-8
to shut up about -Wstringop-truncation warnings, and is intended
to mark those character arrays that are not expected to be
null-terminated but still used with strncpy().
>> spin_unlock(&mp->m_sb_lock);
>> /* xfs on-disk label is 12 chars, be sure we send a null to user
>> */
>> label[XFSLABEL_MAX] = '\0';
>> - if (copy_to_user(user_label, label, sizeof(sbp->sb_fname)))
>> + if (copy_to_user(user_label, label, sizeof(label)))
>
>
>
> ok. (odd how this is ok for copy_to_user but not for strncpy above) :)
No idea. Maybe the gcc bug only happens with struct members but
not local variables?
>> return -EFAULT;
>> return 0;
>> }
>> @@ -1860,7 +1860,7 @@ xfs_ioc_setlabel(
>> spin_lock(&mp->m_sb_lock);
>> memset(sbp->sb_fname, 0, sizeof(sbp->sb_fname));
>> - strncpy(sbp->sb_fname, label, sizeof(sbp->sb_fname));
>> + memcpy(sbp->sb_fname, label, len);
>
>
> Hm but len = strnlen(label, XFSLABEL_MAX + 1);
> which could be one longer than sbp->sb_fname, no?
We have an explicit check for that, so I think it's ok:
if (len > sizeof(sbp->sb_fname))
return -EINVAL;
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
