Commit ad2bd7e0e9647cd48593a6b3a2be07dc2c2d28ed
Committed by
Linus Torvalds
1 parent
fc148a5f7e
Exists in
master
and in
4 other branches
mm/swapfile.c: fix swapon size off-by-one
There's an off-by-one disagreement between mkswap and swapon about the meaning of swap_header last_page: mkswap (in all versions I've looked at: util-linux-ng and BusyBox and old util-linux; probably as far back as 1999) consistently means the offset (in page units) of the last page of the swap area, whereas kernel sys_swapon (as far back as 2.2 and 2.3) strangely takes it to mean the size (in page units) of the swap area. This disagreement is the safe way round; but it's worrying people, and loses us one page of swap. The fix is not just to add one to nr_good_pages: we need to get maxpages (the size of the swap_map array) right before that; and though that is an unsigned long, be careful not to overflow the unsigned int p->max which later holds it (probably why header uses __u32 last_page instead of size). Why did we subtract one from the maximum swp_offset to calculate maxpages? Though it was probably me who made that change in 2.4.10, I don't get it: and now we should be adding one (without risk of overflow in this case). Fix the handling of swap_header badpages: it could have overrun the swap_map when very large swap area used on a more limited architecture. Remove pre-initializations of swap_header, nr_good_pages and maxpages: those date from when sys_swapon was supporting other versions of header. Reported-by: Nitin Gupta <ngupta@vflare.org> Reported-by: Jarkko Lavinen <jarkko.lavinen@nokia.com> Signed-off-by: Hugh Dickins <hugh.dickins@tiscali.co.uk> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Showing 1 changed file with 18 additions and 13 deletions Side-by-side Diff
mm/swapfile.c
... | ... | @@ -1760,11 +1760,11 @@ |
1760 | 1760 | unsigned int type; |
1761 | 1761 | int i, prev; |
1762 | 1762 | int error; |
1763 | - union swap_header *swap_header = NULL; | |
1764 | - unsigned int nr_good_pages = 0; | |
1763 | + union swap_header *swap_header; | |
1764 | + unsigned int nr_good_pages; | |
1765 | 1765 | int nr_extents = 0; |
1766 | 1766 | sector_t span; |
1767 | - unsigned long maxpages = 1; | |
1767 | + unsigned long maxpages; | |
1768 | 1768 | unsigned long swapfilepages; |
1769 | 1769 | unsigned char *swap_map = NULL; |
1770 | 1770 | struct page *page = NULL; |
... | ... | @@ -1923,9 +1923,13 @@ |
1923 | 1923 | * swap pte. |
1924 | 1924 | */ |
1925 | 1925 | maxpages = swp_offset(pte_to_swp_entry( |
1926 | - swp_entry_to_pte(swp_entry(0, ~0UL)))) - 1; | |
1927 | - if (maxpages > swap_header->info.last_page) | |
1928 | - maxpages = swap_header->info.last_page; | |
1926 | + swp_entry_to_pte(swp_entry(0, ~0UL)))) + 1; | |
1927 | + if (maxpages > swap_header->info.last_page) { | |
1928 | + maxpages = swap_header->info.last_page + 1; | |
1929 | + /* p->max is an unsigned int: don't overflow it */ | |
1930 | + if ((unsigned int)maxpages == 0) | |
1931 | + maxpages = UINT_MAX; | |
1932 | + } | |
1929 | 1933 | p->highest_bit = maxpages - 1; |
1930 | 1934 | |
1931 | 1935 | error = -EINVAL; |
1932 | 1936 | |
1933 | 1937 | |
1934 | 1938 | |
... | ... | @@ -1949,22 +1953,23 @@ |
1949 | 1953 | } |
1950 | 1954 | |
1951 | 1955 | memset(swap_map, 0, maxpages); |
1956 | + nr_good_pages = maxpages - 1; /* omit header page */ | |
1957 | + | |
1952 | 1958 | for (i = 0; i < swap_header->info.nr_badpages; i++) { |
1953 | - int page_nr = swap_header->info.badpages[i]; | |
1954 | - if (page_nr <= 0 || page_nr >= swap_header->info.last_page) { | |
1959 | + unsigned int page_nr = swap_header->info.badpages[i]; | |
1960 | + if (page_nr == 0 || page_nr > swap_header->info.last_page) { | |
1955 | 1961 | error = -EINVAL; |
1956 | 1962 | goto bad_swap; |
1957 | 1963 | } |
1958 | - swap_map[page_nr] = SWAP_MAP_BAD; | |
1964 | + if (page_nr < maxpages) { | |
1965 | + swap_map[page_nr] = SWAP_MAP_BAD; | |
1966 | + nr_good_pages--; | |
1967 | + } | |
1959 | 1968 | } |
1960 | 1969 | |
1961 | 1970 | error = swap_cgroup_swapon(type, maxpages); |
1962 | 1971 | if (error) |
1963 | 1972 | goto bad_swap; |
1964 | - | |
1965 | - nr_good_pages = swap_header->info.last_page - | |
1966 | - swap_header->info.nr_badpages - | |
1967 | - 1 /* header page */; | |
1968 | 1973 | |
1969 | 1974 | if (nr_good_pages) { |
1970 | 1975 | swap_map[0] = SWAP_MAP_BAD; |