[LU-4232] MDT and OST should validate incoming FIDs belong to local target

• Status: Open
• Project: Lustre
• Affects Version/s: Lustre 2.1.6, Lustre 2.6.0
• Type: Improvement
• Priority: Critical
• Reporter: Andreas Dilger
• Assignee: WC Triage
• Resolution: Unresolved
• Labels: medium, ppc

Description

Problems in LU-4226 indicate that the MDS allows incoming file creates with bad FIDs to proceed, even though the FID SEQ is not in a valid range. These requests should be refused by the server.

The source of the invalid SEQ may be related to PPC clients not swabbing the allocated SEQ range correctly, but that is the topic of another bug.

Issue Links

• is related to: LU-4226 MDS unable to locate swabbed FID SEQ ...
• is related to: LU-3569 Use real OST index as ostid_to_fid() ...
• is related to: LU-5369 servers should reject invalid FIDs, r...
• is related to: LU-4233 Verify PPC FID swabbed correctly duri...

Comments

[Andreas Dilger]

LU-4233 was filed for the PPC client swabbing issue.

It may be that this checking has already been implemented for 2.4+ MDS due to local/remote checking for DNE. Having a sanity.sh test using a new OBD_FAIL_ | CFS_FAIL_RAND that randomly injects a bad FID SEQ on the client (maybe in mdc_pack_op_data()?) and has it try a bunch of different operations (e.g. by running racer) would be useful.

The FID SEQ should also be verified for OST operations. This was not possible with FID_SEQ_OSD_MDT0 previously, but is possible for FID_SEQ_IDIF and FID_SEQ_NORMAL objects. That is more motivation for http://review.whamcloud.com/7053 from LU-3569 to be landed.

[Andreas Dilger]

At a minimum this needs an audit of the MDT and OST code to verify that incoming code paths (in particular create) has valid FIDs for the target. It may be that 2.4+ already has sufficient checking for this, in which case only a test is needed that forces a client to try and create some files with invalid FIDs (which should fail).

[Andreas Dilger]

Mike, I think that this is already done for OST object FIDs/ostids, and I think that it is done for MDT object FIDs also. Could you please check the code and confirm? If yes, then this bug can be closed.

[Mikhail Pershin]

Andreas, there is check fid_is_sane() for incoming FIDs on MDT, this should prevent the processing of swabbed FIDs. Meanwhile I am not sure about original meaning for this ticket, it says 'incoming FIDs belong to local target', does that mean FID must always be local, i.e. all incoming FIDs must be not remote objects?

[Andreas Dilger]

Mike, the original problem seen was that with older PPC clients they did not swab the incoming SEQ properly and then generated FIDs with bad SEQ to the MDS. If the MDS checked that the FID was local it would have returned an error to the client and we would have noticed and fixed that easily.

[Mikhail Pershin]

yes, and fid_is_sane should catch such FIDs or you mean that doesn't help because only seq is no swabbed and looks like some other big sequence?
If we need to ensure FID is local for that particular MDT it would be more complex. For this we need to ask FLDB or get lu_object by that FID and check it is local.

[Jodi Levi (Inactive)]

Would it make sense to open a new ticket for the more complex work you mention and close this ticket?

[Andreas Dilger]

Mike, I thought Di implemented a local FLDB cache, so doing one FLDB lookup per RPC to verify the FID is local should be ok?

Jodi, I don't know if there is a benefit to close this ticket and open a new one? Are there any patches landed with this ticket? If not, it could just be moved to 2.7. I think the critical checks of this ticket are already present in 2.6, landed under other tickets.

[Andreas Dilger]

Also note that fid_is_sane() only checks if a FID might be OK, but will let almost any value through, and is hardly a check at all. It does nothing to verify if the FID belongs to the client using it to create a new file, or if it belongs to the local server node.