[LU-1853] 'out-of-bounds access' error in osd_scrub_setup()

• Type: Technical task
• Resolution: Won't Fix
• Priority: Major
• Affects Version/s: Lustre 2.4.0
• Labels: build coverity
• Assignee: nasf (Inactive)
• Reporter: Sebastien Buisson (Inactive)

Description

In osd_scrub_setup(), id variable is of type 'struct osd_inode_id *' (size 8). But then this variable is passed to __osd_oi_lookup(), and in this function, it is passed to osd_oi_iam_lookup(), which casts it to a 'struct dt_rec *'. Then it is passed to osd_fid_unpack() by casting it to 'struct lu_fid *'. And at last, inside this function, a memcpy() is performed considering the variable is of type 'struct lu_fid *', which is of size 16.
So we are accessing memory beyond what was allocated for that variable.

I do not know how to fix this issue, as it is quite complex.

Comments

[nasf (Inactive)]

It will not cause issues under current use mode.

[nasf (Inactive)]

Yes, there are some dead codes in IAM, not only for this, but also for LVAR part. But nobody knows whether they will be re-used in the future or not, so these codes keep there since the beginning.

[Sebastien Buisson (Inactive)]

OK I understand.
So maybe in that case, it would be better to remove this 'dead' branch, right?

[nasf (Inactive)]

Originally, the IAM was designed not only for index file, such as OI files, and quota files, but also for storing directory under Lustre namespace. But the later use case is not valid since Lustre-2.0, that means the "if (S_ISDIR(oi->oi_inode->i_mode))" branch will not be trigger anymore in current 2.x code. So I think there will not be out-of-bounds" issues in IAM according to current use cases.

[Sebastien Buisson (Inactive)]

Hi Yong Fan,

This issue was found by the static code analysis tool named Coverity.

According to your analysis, osd_fid_unpack() will not be called because the condition above will evaluate to false. But maybe there is another call path leading to the issue described here?

Sebastien.

[nasf (Inactive)]

static int osd_oi_iam_lookup(struct osd_thread_info *oti,
                             struct osd_oi *oi, struct dt_rec *rec,
                             const struct dt_key *key)
{
...
        rc = iam_it_get(it, (struct iam_key *)key);
        if (rc >= 0) {
                if (S_ISDIR(oi->oi_inode->i_mode))
                        iam_rec = (struct iam_rec *)oti->oti_ldp;
                else
                        iam_rec = (struct iam_rec *)rec;
                
                iam_reccpy(&it->ii_path.ip_leaf, (struct iam_rec *)iam_rec);
===>                if (S_ISDIR(oi->oi_inode->i_mode))
                        osd_fid_unpack((struct lu_fid *)rec,
                                       (struct osd_fid_pack *)iam_rec);
        }               
...
}

In this case, the IAM container is an OI index file, which is NOT directory, so the branch for "if (S_ISDIR(oi->oi_inode->i_mode))" in above code section will be skipped for the call trace: osd_scrub_setup() => __osd_oi_lookup() => osd_oi_iam_lookup(). So the OI scrub should not trigger "out-of-bounds access" error.

Sebastien, have you hit such failure during any test? or thought there may be potential issues by analyzing code?