Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] fs/jfs: fix potential integer overflow on shift of a int
To:     Colin King <colin.king@canonical.com>,
        Tino Reichardt <milky-kernel@mcmilk.de>,
        jfs-discussion@lists.sourceforge.net
Cc:     kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20210205171137.208121-1-colin.king@canonical.com>
From:   Dave Kleikamp <dave.kleikamp@oracle.com>
Message-ID: <044b0372-8612-6fb9-23c5-9c4e6ab79114@oracle.com>
Date:   Tue, 9 Feb 2021 10:52:32 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.7.0
In-Reply-To: <20210205171137.208121-1-colin.king@canonical.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from [192.168.0.190] (68.201.65.98) by SA0PR11CA0162.namprd11.prod.outlook.com (2603:10b6:806:1bb::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3846.25 via Frontend Transport; Tue, 9 Feb 2021 16:52:33 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 4ce5dd46-f579-4492-b5e5-08d8cd1b1927
X-MS-TrafficTypeDiagnostic: CH2PR10MB4232:
X-Microsoft-Antispam-PRVS: <CH2PR10MB42323C4251B6E36110392DDC878E9@CH2PR10MB4232.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:4941;
X-MS-Exchange-SenderADCheck: 1
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: X6qTC5CzBTby6Vz0EEiAJ1OysXtdDC0QZ5zHg/fbHV65eh/Wnq4bZwR/JRv1TjiKsPxUnL+2XlCzUEihAXohBqKWCOF5oanTnZhvGzklmZY6ClyRdDMf5fsoyoj+1llAyUk16M+kdrtD4aqJh1C+UU/TSMhjmrwlsGgYhM0YKx+XOOOLvyPHk1s7UzJIsoCI0OIAq4IRTXGFdjcX4fmtZQd2KCjQwXhDDlbWdcSWgfJ8ObHbinY+UZgq9LmILSGnE7xVDmsy52/2G2KSz8Fes8iHXpqiz29s/omboE3KDCUoz1QFx+Xb5EEHjPgDvrMHtEZBz+d532VRL9mZG+1nEs7Wy+XhH+bmzb56T1MVSJmcwcEfXLcJKftgYSqM1MoE6eHo82szUjPGpZc/hv71cG4g8pV4F63VHWFn4tn3WkHqnYqmWv1J6tHRZAgyMCCjeEcjPfbIsQHXhxXJjkgCh1tf2OgLQkJzQnpXN67gmGQN22IJFCTa1yONstOcY+aqJ4c4bR/47BpuybpHWYZDgHlVgsZgMMtHLAOLx4G0ii8zMwIqsRLvr7YaCRlHLpXqgEEYJHel9qS9QT/gER+TdcrGbiqsel6aTsnBBh9OIo4=
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CH2PR10MB4118.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(39860400002)(346002)(136003)(376002)(396003)(366004)(66556008)(66476007)(26005)(31696002)(478600001)(83380400001)(2906002)(2616005)(44832011)(186003)(956004)(66946007)(31686004)(16576012)(316002)(16526019)(53546011)(8676002)(4326008)(86362001)(110136005)(5660300002)(36756003)(6486002)(8936002)(45980500001)(43740500002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?ZElwN29RUTRUYVVyUmVKNW9nRDBtRHp6TUpRQ3hMUXltR052ckUrZ2p6dkpE?=
 =?utf-8?B?OWRDckdseG1TMU9qcEkwVXh2OWNOczJYM0N4eW9ObWEya1lxYnFOWWdBdEo2?=
 =?utf-8?B?YWpqZk5LdllIR2xuTkxPN3lUcnI4dVZ4bVNaQlpZN3ZBKzhGa2o4VXBXZ3lZ?=
 =?utf-8?B?RHRlOWVULytLb2lGckJaZVl5cWVKTGVRWWg4WWxLMks3NGF6cWl2aXhLdW5l?=
 =?utf-8?B?ZFdSMWZDbExQbjhFblpaSHpKN2hHRGVuSnVNOGE3WE1TSHd6cy9wbThobUww?=
 =?utf-8?B?Z283bURXM1NJOUdaUko0Z1ExY1VkWUxNd21NOUpROHdocTRLNlBuR3NndW1P?=
 =?utf-8?B?M2pOMXlpajdWajU2cEVpeWlNTjhBbnZvUGV3TUpRSW8yOWpQZ09PMGhpR0VQ?=
 =?utf-8?B?dzdienBtVlV1UXY3eDR5R2JscXpUTW04ODdLdHR4VGlnaUNmOTR4c2VVQjA3?=
 =?utf-8?B?aEZZOTV5MGNYM3NFNWdTOHUrWGpZYzd6OVFWUytnZmFGcE85bTArMENNbWl4?=
 =?utf-8?B?bUFKSHd4RUQxUHRPTldZRnVySy9MUVE2eDVlVDlXYjFmZk9RUndPa0RwRjJv?=
 =?utf-8?B?dHlsVHk5elhzczVoRlg2QjlUeEJiUm9tMEVzcEJsZE1PZGYvdnozZXUyWGZI?=
 =?utf-8?B?NFFOLzNNd1p4clFLT0NQSCtqWGpQMmtHSHVhOFdxa3RsZmVxTDZyL25KQnBp?=
 =?utf-8?B?N1IycjBXOVIwSkZMd0tzaDZYcGFNd3lQSkN2MFFtZ2ZaV2VjWDV4V0FVVDRF?=
 =?utf-8?B?djVRd3JNeUU1SEtnVk96YzZ2SDN5N0V6NVQvUDhXUFZBaXdNVkpCSDVCNmtt?=
 =?utf-8?B?eHkrK3NFcDZlN00wZ2U2NndDc0JUOGQ5VFlyci9pZGdCbGRaUnFkTWcvOXFt?=
 =?utf-8?B?NHdhNlJ3alNhTTd6YUkzR2lSdGNVV2phQ2Y3TDlISFNPdjlMVG9QV21idnJ3?=
 =?utf-8?B?dXVadkZubkxDVlJYOUZ6ZldhWElaZXpOUDE4UHBoaGt1OFRxR0FqNFRUelhx?=
 =?utf-8?B?NHZsWm4zTklDR2t5MG9yZVVWSzMwMWRjMHljV0l4OGV1YW90TVZVeldDYjRM?=
 =?utf-8?B?ME4vRnhCUjlUY0ZsZ3VHdVg0R3BRUWI5Ui94Y0k1QUtTcFZRRXduNEttQ2Z0?=
 =?utf-8?B?aFN6aEJ2VDh1aUhWWUcya1BTODJxeDk2QzFwZ1p6QmRDZUN5ei9nbnNqVkJz?=
 =?utf-8?B?UGhpK1UrTUVlNERRNmp5MjlQY3YxQTJ0VTBBRUw3VmNManFwZjF6VmlhelF5?=
 =?utf-8?B?RlRaUmcvaGJtaVRlME1qZjA2RkdLZWgxVDZJYy91Z0REazJmOGxrcmVSanZT?=
 =?utf-8?B?VHUwa2hLb1hjYTlvR2hrMGtLMlVrRVRNYXN0VEMrOWlWUktvOGxHT3h0Y3lO?=
 =?utf-8?B?MlREMDVvWW9CdEJselhwTkIwazUzdVc5UWlrSjVxa3BoazJsWDY4Qmh3cEE0?=
 =?utf-8?B?SlZzMkxMKzBDaktpRko0b09qWnRrZFcxWXlzN1RIdUxScC84MjRDbHRDT3Ri?=
 =?utf-8?B?OTl6bjNONkM2UURvQ0FWRDBIZXlRTkN6L1MxcHVlWnVuTUR6VVA3WUw1cUZ5?=
 =?utf-8?B?ZUVxU1Jtc1RnRW44OTVsMXlpUlJFQXV4WTlzN1RIWmVWQ2lweDlKQjMwS04x?=
 =?utf-8?B?UGgvbStucGg3Z3c1dWV6Q2x6NkkycVMvNFRZdWZNRFlKcERtVERFYjBiaVZk?=
 =?utf-8?B?VHpZRE0wVmJqVHlocitxQmNnL1RVYkdOVUQwNm1SVENrRzZzR2hIaW1kb0N3?=
 =?utf-8?Q?yPYP3vf9WO/yrCqofyPgqLOJTssv7a+9AC7yzOM?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4ce5dd46-f579-4492-b5e5-08d8cd1b1927
X-MS-Exchange-CrossTenant-AuthSource: CH2PR10MB4118.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2021 16:52:34.5228
 (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: LuESIHbJCDiJFZD22wmt+YmXxM8cqHnDVkd4U7rP74sHvT2nP85RjYJJo9XzuiDuV1R+Q8Qnk1q5CGofgp0tb97X39+spzlsZrJbgVIL1Y8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR10MB4232
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9890 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 bulkscore=0 adultscore=0
 mlxlogscore=999 phishscore=0 spamscore=0 suspectscore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2102090082
X-Proofpoint-Virus-Version: vendor=nai engine=6200 definitions=9890 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 priorityscore=1501 malwarescore=0
 spamscore=0 lowpriorityscore=0 phishscore=0 adultscore=0 impostorscore=0
 suspectscore=0 mlxscore=0 clxscore=1011 mlxlogscore=999 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000
 definitions=main-2102090082
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2/5/21 11:11 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The left shift of int 32 bit integer constant 1 is evaluated using 32 bit
> arithmetic and then assigned to a signed 64 bit integer. In the case where
> l2nb is 32 or more this can lead to an overflow.  Avoid this by shifting
> using the BIT_ULL macro instead.
> 
> Addresses-Coverity: ("Uninitentional integer overflow")
> Fixes: b40c2e665cd5 ("fs/jfs: TRIM support for JFS Filesystem")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   fs/jfs/jfs_dmap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_dmap.c b/fs/jfs/jfs_dmap.c
> index 94b7c1cb5ceb..47dbca7e52e0 100644
> --- a/fs/jfs/jfs_dmap.c
> +++ b/fs/jfs/jfs_dmap.c
> @@ -1656,7 +1656,7 @@ s64 dbDiscardAG(struct inode *ip, int agno, s64 minlen)
>   		} else if (rc == -ENOSPC) {
>   			/* search for next smaller log2 block */
>   			l2nb = BLKSTOL2(nblocks) - 1;
> -			nblocks = 1 << l2nb;
> +			nblocks = BIT_ULL(l2nb);

I'm not sure I like the use of this macro here. It seems to imply a bit 
flag of some sort. I think it would be clearer to use

			nblocks = 1ULL << l2nb;

Maybe 1LL rather than 1ULL since nblocks is s64.

>   		} else {
>   			/* Trim any already allocated blocks */
>   			jfs_error(bmp->db_ipbmap->i_sb, "-EIO\n");
>