[LU-4984] Integer overflow in LL_IOC_HSM_REQUEST handler

• Type: Bug
• Resolution: Fixed
• Priority: Critical
• Fix Version/s: Lustre 2.6.0
• Affects Version/s: Lustre 2.5.0, Lustre 2.6.0
• Labels: mq115
• Severity: 3
• Assignee: Nathaniel Clark
• Reporter: Oleg Drokin

Description

There's an integer overflow in LL_IOC_HSM_REQUEST handler

        case LL_IOC_HSM_REQUEST: {
                struct hsm_user_request *hur;
                int                      totalsize;

                OBD_ALLOC_PTR(hur);
                if (hur == NULL)
                        RETURN(-ENOMEM);

                /* We don't know the true size yet; copy the fixed-size part */
                if (copy_from_user(hur, (void *)arg, sizeof(*hur))) {
                        OBD_FREE_PTR(hur);
                        RETURN(-EFAULT);
                }

                /* Compute the whole struct size */
                totalsize = hur_len(hur);
                OBD_FREE_PTR(hur);

                /* Make sure the size is reasonable */
                if (totalsize >= MDS_MAXREQSIZE)
                        RETURN(-E2BIG);

Instead of checking totalsize which is past multiplication and is subject to overflow already, what we must do is we must ensure hur->hur_request.hr_itemcount is safe first.
Then it's safe to call hur_len

Issue Links

• is related to: LU-5323 memory leak in lfs_hsm_request()

Comments

[Oleg Drokin]

The leak is in a short-lived lfs tool, not any sort of a library that can for extended periods of time, so I don't see this is a very important leak (still a bug, of course).

Please open a separate ticket about it so that it's not forgotten.

[Frank Zago (Inactive)]

The committed patch still has the memory leak mentioned here: http://review.whamcloud.com/#/c/10615/5/lustre/utils/lfs.c,cm

[Peter Jones]

Landed for 2.6

[Nathaniel Clark]

http://review.whamcloud.com/10615

[Andreas Dilger]

John,
is there any chance you would have time to look at this?

[Andreas Dilger]

John, I suspect we should also annotate some of these fields with __user as well, could you please comment.

[Jodi Levi (Inactive)]

James,
Could you please take this one?
Thank you!