Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756142AbdIGVVx (ORCPT ); Thu, 7 Sep 2017 17:21:53 -0400 Received: from mx0a-00190b01.pphosted.com ([67.231.149.131]:56575 "EHLO mx0a-00190b01.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755911AbdIGVVv (ORCPT ); Thu, 7 Sep 2017 17:21:51 -0400 Subject: Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove the restore_c_regs_and_iret label) To: Linus Torvalds Cc: Ingo Molnar , Igor Lubashev , Josh Hunt , Pablo Neira Ayuso , Borislav Petkov , Andy Lutomirski , the arch/x86 maintainers , Linux Kernel Mailing List , Brian Gerst , Andrew Cooper , Juergen Gross , Boris Ostrovsky , Kees Cook , Andrew Morton , "David S. Miller" , Arnd Bergmann References: <69b38985-e094-ddcc-6f7e-d6e5cc2c657e@akamai.com> <6667f710-68f3-b97e-b0eb-d9879476831e@akamai.com> From: Vishwanath Pai Message-ID: Date: Thu, 7 Sep 2017 17:21:19 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-09-07_12:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 suspectscore=0 malwarescore=0 phishscore=0 adultscore=0 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1709070316 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:,, definitions=2017-09-07_12:,, signatures=0 X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0 suspectscore=0 phishscore=0 bulkscore=0 spamscore=0 clxscore=1015 lowpriorityscore=0 impostorscore=0 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.0.1-1707230000 definitions=main-1709070316 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1507 Lines: 45 On 09/07/2017 04:45 PM, Linus Torvalds wrote: > On Thu, Sep 7, 2017 at 1:16 PM, Vishwanath Pai wrote: >> >> Writing U32INT_MAX as 0xFFFFFFFFULL was a mistake on my part. I could >> have avoided all of this by using built-in constants instead of trying >> to define them myself. I will rewrite the function as below and send out >> another patch: >> >> static u64 user2rate_bytes(u64 user) >> { >> u64 r; >> >> r = user ? U32_MAX / (u32) user : U32_MAX; >> r = (r - 1) << XT_HASHLIMIT_BYTE_SHIFT; >> return r; >> } > > No, that is *still* wrong. > > In particular, the test for "user" being zero is done in 64 bits, but > then when you do the divide, the cast to (u32) will take the low 32 > bits - which may be zero, because only upper bits were set. > > So now you get a divide-by-zero. > > What seems to be going on is that a value larger than UINT32_MAX is > basically "invalid", since the reverse function cannot possibly > generate that. > > So one possible fix is to just make that an error case in the caller, > and then make user2rate_bytes() not take (or return) "u64" at all, but > simply use u32. > > Please be more careful here. > > Linus > Yes, that is true. Thanks for pointing it out. I will change the user param to 'u32', and also change the return type to u32 as well. I will add a check in hashlimit_mt_check() to make sure the userspace never sends anything > U32_MAX and error out if they do. Thanks, Vishwanath