On 4/4/18 10:52 PM, Darrick J. Wong wrote:
> On Wed, Apr 04, 2018 at 01:54:26PM -0500, Eric Sandeen wrote:
>> In order to validate the UUID in xfs_dquot_verify, we need
>> the full xfs_qblk, not just the xfs_disk_dquot_t (which is
>
> ^^^^^^^^^ xfs_dqblk, right?
yup
...
>> @@ -192,14 +191,10 @@
>
> Any way you can get your diff generator to add -p to spit out the
> alleged function this chunk is supposed to land in? It makes reviewing
> patches somewhat easier for me. :)
No doubt ... I don't know why it doesn't do so, sorry. :/ Will try
to figure that out. Sorry about that.
>> * buffer so corruptions could point to the wrong dquot in this case.
>> */
>> for (i = 0; i < ndquots; i++) {
>> - struct xfs_disk_dquot *ddq;
>> -
>> - ddq = &d[i].dd_diskdq;
>> -
>> if (i == 0)
>> - id = be32_to_cpu(ddq->d_id);
>> + id = be32_to_cpu(d[i].dd_diskdq.d_id);
>>
>> - fa = xfs_dquot_verify(mp, ddq, id + i, 0);
>> + fa = xfs_dquot_verify(mp, &d[i], id + i, 0);
>> if (fa)
>> return fa;
>> }
...
>> @@ -1013,9 +1018,6 @@
>> return -EIO;
>> }
>>
>> - /* This is the only portion of data that needs to persist */
>> - memcpy(ddqp, &dqp->q_core, sizeof(xfs_disk_dquot_t));
>
> About this memcpy() -- isn't the point of this function that we verify
> the contents of the in-core q_core and only memcpy the contents into the
> xfs_buf if it actually passes validation?
yeah, but if it fails here we release the buffer & shut down the filesystem ;)
> I guess the _dquot_verify
> function needs the entire on-disk buffer so that it can validate the crc
narrator: xfs_dquot_verify doesn't verify the crc ;)
> and the uuid on a read, but we update the crc on dqflush and
> (presumably) can set the uuid on write (quotacheck) or fail the dquot
> read everywhere else, right?
>
> Put another way, why not have xfs_dquot_buf_verify check the uuid?
> xfs_dquot_repair seems to reset the uuid if it's garbage.
Well, the above path (xfs_qm_dqflush) isn't going to do repair...
But OK, xfs_dquot_buf_verify does the entire dqblk; it iterates over
each dquot calling xfs_dquot_verify. I figured the easiest way to
get uuid validation was to put it into xfs_dquot_verify.
But I guess you're suggesting a separate uuid check in xfs_dquot_buf_verify
to validate the uuid, given that it has the full on-disk dquot? Ok that
might make sense...
Thanks,
-Eric
--
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
