Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <95d1dc4e-fc3b-fe3c-5d85-218a7410e966@oracle.com>
Date:   Wed, 9 Feb 2022 16:43:29 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.5.0
Subject: Re: [PATCH] hugetlbfs: fix a truncation issue in hugepages parameter
Content-Language: en-US
To:     liuyuntao <liuyuntao10@huawei.com>, akpm@linux-foundation.org,
        yaozhenguo1@gmail.com
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        wuxu.wu@huawei.com, fangchuangchuang@huawei.com,
        windspectator@gmail.com
References: <20220209134018.8242-1-liuyuntao10@huawei.com>
From:   Mike Kravetz <mike.kravetz@oracle.com>
In-Reply-To: <20220209134018.8242-1-liuyuntao10@huawei.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?VlNqZCsvV29yMnlmNjd1Y3dOQW1oOW5OOFdidHVXRnRkUng4cXpoRXJhWkI5?=
 =?utf-8?B?SEdCRkZWLzdQd2NvNlZvVjVRdkNTSWcxUWtDNDdjL3pMTXVnM0xyZXFOZjZE?=
 =?utf-8?B?OFN2MXdmWFVOUFFMNGoyS0ZzNitUOFJjaFRKZWZDL2xHOFR0SlZJN004UURs?=
 =?utf-8?B?cDc1VWx3b0I3NndiWEZ3VnNpeFRVMm9heFNPTzJaRkM2NHVtdDAvYk9NUzVt?=
 =?utf-8?B?MHBsVENaUkFVTkNRaDlSRktRMXhnczB2bWlHUVJTQnppbURNOER3RHgyL2VV?=
 =?utf-8?B?a1pnbGNmL2Z6VTRKdjJNVkxhQlBMUUJpNkpuNm9mV0tLSThncWxTM2kycTdv?=
 =?utf-8?B?MlVuM2FxS0VFWEhjQUk2eVp6MUI5WUx0RHJxeldYblVDb2pNUkg2OWMzTkJB?=
 =?utf-8?B?QkdabDdTZHV5TVNhWkphYlJtUThOMllBYTd1SGJvODBNM2dCTmE5a2pISDFR?=
 =?utf-8?B?SDVpZ0Q1UnNWUm9oOHlFUFZ5eDdtZmlWQ3FOby96OXREeExyU0tVZTE4Z2ph?=
 =?utf-8?B?T1RreE45OHJVczNHS0NSQTF2YUxWQitWTlRwS3NNdlNlYzlibDAveXEvZHgw?=
 =?utf-8?B?VnhFVzROellCd215em1zeWlyMEVzanVlRmp3bFBOSDlmRXRSVStVVVZkVUZB?=
 =?utf-8?B?bEpwT3ZKNlZhQU1ueEFDS0NhajBpaG9zN3g0ZTBNSjJldVpYV0R6K2VyZHh3?=
 =?utf-8?B?MW1yZWRGTVFNUWZhcmFXNVF5NktxOElRTTF5Qkk3WUlpaDEzaTMyMHFiSk1D?=
 =?utf-8?B?YTd1QzB4Z21tTTh3bVA1M2IrWU9RTEdESGtjTU14RHVjMG9BOS9BUVB2M1VU?=
 =?utf-8?B?NklvNlBocW1FNkwzdWxwY1V1NmRWbVJ4QzNpSTYrU1lCVlNDOVc5clRIYzg2?=
 =?utf-8?B?bUZ2Y0lPR05OUVFxMldiVU05ZldjL0xubE4xN1NlZ3JOYzlobXpxQ1hFYnB5?=
 =?utf-8?B?OERKK1JnTUZGR2Q2VVhDZUhKZFFoKzJONzFxK3BwSyt3TStDWnR3RFdHMmhi?=
 =?utf-8?B?djRBeGUxaFlUV29KbTUzWGsva0RTSnY4YnFlZGE4K1NJazZiYndrdzg0N0Zi?=
 =?utf-8?B?aURKS3RtZXg5Zmh5T01FSW9mZGZTMmluWElSNnhWdFc2eWljZ0IxcTBEMWMz?=
 =?utf-8?B?MUtwWVFOeStzTHYrcEhvQmZVZmZobWhFbmFSSStscVJtejg4MUR3ZXJlckNL?=
 =?utf-8?B?NFFnUFJDTkVZQll5alVaN2QwRjF4RDVDZm9iMVYxVmcyMllNSnRlK2VaVUZ6?=
 =?utf-8?B?eWZIUUVubFBsQXZZN2RSTlNKRTNqQ2M4Ri9WbTdmbUE0NW9Zbm5ZVTlqVkhK?=
 =?utf-8?B?Rm9EaHRKZkhDWUEvbkZMUy96OW9IVkE1ZUlpV1RFcjEyWWNYSHJTeCtoSlgw?=
 =?utf-8?B?cTZJSGVoejYrZmxTNHR4VEVNMzFDRXA2Q2ViYTlhaU5DTlRmZWZhZWhxdkMx?=
 =?utf-8?B?UGg3R20rSkNxM1oxYlRCVmw4ZHNyRTVPYTI2UGtJNk9GUU9kRzBwZmQvY3pr?=
 =?utf-8?B?d1RueDdTYkR3TnJLV0VsSjVZNTVEY1FOMDhGYlhwZ3lJYVhVdHJlMUtHd1Vq?=
 =?utf-8?B?Vk55Tm8zVVZFclk0S3dJVmpUaExUb2ZnbGhyR2U2cFB6SU9WVWg3VG1vSkk3?=
 =?utf-8?B?dTZpM3VIbXVyQzFQZzRCa0t6ZmM4ZnJudUkvaHg3YTZ6LzFwajZQenBLVzg0?=
 =?utf-8?B?UHNKUmlHQmFuN29CNEk1ZGl2QnNwRTJ0MFgwekU2cmcvbi9XL0RGYzNUYyt1?=
 =?utf-8?B?RVJRWXhKenNJempNTmRtWTJmMzQ5Q3cvWUlCc1dhTFpMMHd6a2R0azFleUdp?=
 =?utf-8?B?cENhOVJ2TVFRRHpwdUJOa01KTDh0dFFPTk9Ub3JrWVNwNURTT3U0cUx1MTlZ?=
 =?utf-8?B?QkZvWVFnSWJva1U1UXRpbTB4VnpUR0NSV2lheiszY1lGZU5Mb1FjdEdRWHNo?=
 =?utf-8?B?U3cvalNPbHY1OWtYZFVnSzQ3Wjd2MGJ0OXJlOW43cmZGbk5zNzNXUVpsUy9H?=
 =?utf-8?B?WGEyK2plaGdVb2ZNaTk1M05RNTYxMGtVM1grK2kvTDUzU0NYQXR6bUovc1hH?=
 =?utf-8?B?UHhvdE1XTzExSUlBUGRYUGxkVUFTNEtRMG9RUDRaNVA3YSt3Y0VPVkRybUhs?=
 =?utf-8?B?N2FtN01Lclp0dmUrNGtiSXdCVUtvRmZ0Z3JsblR6VDZuQTJ1UTNtQzMzakFS?=
 =?utf-8?Q?PKPjllZjfPEleGhGkO6Ho94=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 476a8ab9-c58a-4fbc-f4df-08d9ec2e5ce6
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 10 Feb 2022 00:43:32.5001
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: cnFozdSfpJo4MqYDXeW4r6X0OuR3h5cXCMM9UiTKygs6sjoi867vNFCOxdsgh8Akkvqm3/pvHsXK0fKqNhfaRA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR10MB1618
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10253 signatures=673431
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 bulkscore=0 suspectscore=0
 mlxlogscore=999 mlxscore=0 adultscore=0 malwarescore=0 spamscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2202100001
X-Proofpoint-GUID: YFcMPDIVD3oyZ7p04cKSuYdfSg_71PzJ
X-Proofpoint-ORIG-GUID: YFcMPDIVD3oyZ7p04cKSuYdfSg_71PzJ
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2/9/22 05:40, liuyuntao wrote:
> From: Liu Yuntao <liuyuntao10@huawei.com>
> 
> When we specify a large number for node in hugepages parameter,
> it may be parsed to another number due to truncation in this statement:
> 	node = tmp;
> 
> For example, add following parameter in command line:
> 	hugepagesz=1G hugepages=4294967297:5
> and kernel will allocate 5 hugepages for node 1 instead of ignoring it.
> 
> I move the validation check earlier to fix this issue, and slightly
> simplifies the condition here.
> 
> Fixes: b5389086ad7be0 ("hugetlbfs: extend the definition of hugepages parameter to support node allocation")
> Signed-off-by: Liu Yuntao <liuyuntao10@huawei.com>
> ---
>  mm/hugetlb.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 61895cc01d09..0929547f6ad6 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4159,10 +4159,10 @@ static int __init hugepages_setup(char *s)
>  				pr_warn("HugeTLB: architecture can't support node specific alloc, ignoring!\n");
>  				return 0;
>  			}
> +			if (tmp >= nr_online_nodes)
> +				goto invalid;
>  			node = tmp;

I am surprised none of the automated checking complained about that
assignment.

>  			p += count + 1;
> -			if (node < 0 || node >= nr_online_nodes)

I can't remember, but I think that check for node < 0 was added to handle
overflow during the above assignment.  Do you remember Zhenguo Yao?
   
> -				goto invalid;
>  			/* Parse hugepages */
>  			if (sscanf(p, "%lu%n", &tmp, &count) != 1)
>  				goto invalid;

Thanks,

Reviewed-by: Mike Kravetz <mike.kravetz@oracle.com>

-- 
Mike Kravetz