[LU-4984] Integer overflow in LL_IOC_HSM_REQUEST handler Created: 30/Apr/14  Updated: 01/Dec/14  Resolved: 01/Jul/14

Status: Resolved
Project: Lustre
Component/s: None
Affects Version/s: Lustre 2.5.0, Lustre 2.6.0
Fix Version/s: Lustre 2.6.0

Type: Bug Priority: Critical
Reporter: Oleg Drokin Assignee: Nathaniel Clark
Resolution: Fixed Votes: 0
Labels: mq115

Issue Links:
Related
is related to LU-5323 memory leak in lfs_hsm_request() Closed
Severity: 3
Rank (Obsolete): 13804

 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

 Comments   
Comment by Jodi Levi (Inactive) [ 30/Apr/14 ]

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

Comment by Andreas Dilger [ 30/Apr/14 ]

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

Comment by Andreas Dilger [ 05/Jun/14 ]

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

Comment by Nathaniel Clark [ 05/Jun/14 ]

http://review.whamcloud.com/10615

Comment by Peter Jones [ 01/Jul/14 ]

Landed for 2.6

Comment by Frank Zago (Inactive) [ 01/Jul/14 ]

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

Comment by Oleg Drokin [ 10/Jul/14 ]

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.

Comment by Peter Jones [ 10/Jul/14 ]

Oleg

I think that Andreas did this on Frank's behalf under LU-5323

Peter

Comment by James Nunez (Inactive) [ 21/Oct/14 ]

Patch for b2_5 at http://review.whamcloud.com/12369

Generated at Sat Feb 10 01:47:32 UTC 2024 using Jira 9.4.14#940014-sha1:734e6822bbf0d45eff9af51f82432957f73aa32c.