Uploaded image for project: 'Lustre'
  1. Lustre
  2. LU-8057

o2iblnd driver is causing memory corruption due to improper handling of scatter list.

Details

    • Bug
    • Resolution: Fixed
    • Blocker
    • Lustre 2.9.0
    • Lustre 2.7.0, Lustre 2.8.0, Lustre 2.9.0
    • Any installation running Lustre on top of a infiniband stack.
    • 3
    • 9223372036854775807

    Description

      A bug was discovered in the upstream kernel in the handling of the scatter list, tx->tx_frag, in the o2iblnd driver. So the fix of using sg_next was introduced but it revealed a serious bug in that when all 256 pages allocated for fragments are used and the data is at an offset that an extra random page of memory is stomped on.

      Attachments

        Issue Links

          Activity

            [LU-8057] o2iblnd driver is causing memory corruption due to improper handling of scatter list.

            patch has landed to master for 2.9.0

            jgmitter Joseph Gmitter (Inactive) added a comment - patch has landed to master for 2.9.0

            Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/19342/
            Subject: LU-8057 ko2iblnd: Replace sg++ with sg = sg_next(sg)
            Project: fs/lustre-release
            Branch: master
            Current Patch Set:
            Commit: d226464acaacccd240da43dcc22372fbf8cb04a6

            gerrit Gerrit Updater added a comment - Oleg Drokin (oleg.drokin@intel.com) merged in patch http://review.whamcloud.com/19342/ Subject: LU-8057 ko2iblnd: Replace sg++ with sg = sg_next(sg) Project: fs/lustre-release Branch: master Current Patch Set: Commit: d226464acaacccd240da43dcc22372fbf8cb04a6

            The memory corruption exist with the current implementation using sg+. If the offset is not zero and if all 256 pages for the fragments are used then an extra random page gets stomped on. So what we have now made it harder to detect since it was a silent corruption. In the upstream client when sg+ was replaced by sg = sg_next() there was no extra random page you could stomp on so I was seeing failures in my testing. Let me duplicate the failures and post them here.

            simmonsja James A Simmons added a comment - The memory corruption exist with the current implementation using sg+ . If the offset is not zero and if all 256 pages for the fragments are used then an extra random page gets stomped on. So what we have now made it harder to detect since it was a silent corruption. In the upstream client when sg + was replaced by sg = sg_next() there was no extra random page you could stomp on so I was seeing failures in my testing. Let me duplicate the failures and post them here.
            pjones Peter Jones added a comment -

            Doug

            Could you please review this patch?

            Thanks

            Peter

            pjones Peter Jones added a comment - Doug Could you please review this patch? Thanks Peter

            James, could you clarify if the memory corruption issue is related to the use of sg_next(), or if it also existed with the sg++ implementation but was just harder to detect? That will affect which Lustre versions this patch needs to be backported to.

            adilger Andreas Dilger added a comment - James, could you clarify if the memory corruption issue is related to the use of sg_next(), or if it also existed with the sg++ implementation but was just harder to detect? That will affect which Lustre versions this patch needs to be backported to.

            I created a patch that has the back ported patch as well as a possible fix. Its at http://review.whamcloud.com/#/c/19342. The original patch was sent to Greg directly so its not in the driver-devel mailing archives. Using the upstream staging tree you can inspect this change at git commit 3d1477309806459d39e13d8c3206ba35d183c34a

            simmonsja James A Simmons added a comment - I created a patch that has the back ported patch as well as a possible fix. Its at http://review.whamcloud.com/#/c/19342 . The original patch was sent to Greg directly so its not in the driver-devel mailing archives. Using the upstream staging tree you can inspect this change at git commit 3d1477309806459d39e13d8c3206ba35d183c34a
            green Oleg Drokin added a comment -

            Can you please link to the actual details of the problems here?

            green Oleg Drokin added a comment - Can you please link to the actual details of the problems here?

            People

              doug Doug Oucharek (Inactive)
              simmonsja James A Simmons
              Votes:
              0 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: