Received-SPF: pass (google.com: domain of linux-kernel+bounces-158672-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <f1a92175-29e1-467a-9334-9d5a9505660a@oracle.com>
Date: Thu, 25 Apr 2024 09:14:09 -0500
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] jfs: Fix array-index-out-of-bounds in diFree
To: Jeongjun Park <aha310510@gmail.com>, willy@infradead.org
Cc: brauner@kernel.org, jfs-discussion@lists.sourceforge.net,
        jlayton@kernel.org, linux-fsdevel@vger.kernel.org,
        linux-kernel@vger.kernel.org, shaggy@kernel.org,
        syzbot+241c815bda521982cb49@syzkaller.appspotmail.com,
        syzkaller-bugs@googlegroups.com
References: <ZipSO4ITxuy2faKx@casper.infradead.org>
 <20240425141038.47054-1-aha310510@gmail.com>
From: Dave Kleikamp <dave.kleikamp@oracle.com>
Content-Language: en-US
In-Reply-To: <20240425141038.47054-1-aha310510@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?VC95SVU5NTUvdEl6MnBGdExhR1FBU2FKaXAzTVdVSzNhNlp4cXR0dk1EdDhN?=
 =?utf-8?B?c1YyUEsvRU5VNEpUVVI0elZBZnQ3ZUF3ZWkrYmpIcndsU1FwM09pdFFRVGQv?=
 =?utf-8?B?SzI4MEV0UzJEbWlSOHc3a3ZXc3B6dCs3aldoV1lySXJWa3J4TDB5NzhKUFBo?=
 =?utf-8?B?UFR0TVVrSE5Kd1duL0pPTGx1RjNidEVvSllQM3kvcTAxWmtKdXhoMjJwdjBJ?=
 =?utf-8?B?Mm05SDJ0Z3hrZjkyVmJoNVlPV2ZzUlFDQlU2a1pTVUgxdDRPTHhhUi9BOC9I?=
 =?utf-8?B?L3AxTStEN2lzb25QUURBckVDSWYvaFVYT0w4NE9wOGlDMDRmVmtzcXkxMVAr?=
 =?utf-8?B?SDdQdk9LQ1U5T3dsMzgzMDNaN0NCcFpqSmQzbGx0bmIzSzUvbFQ5MkptWHQw?=
 =?utf-8?B?V1hhUmxQL0dDbTA1UGYwTDdjaTNORGMwdzBGOXFQWHR6Q2ZUZVJyclBnUDVs?=
 =?utf-8?B?UU52dzlINUhwM3JPSERjSVRVb0lsNlA4dk1VZTRWZVZ4WG9pUzV6Um5Tdkp4?=
 =?utf-8?B?bEtDMFl5YjNQUUw1dFBhS1dGV243aUxaZ2tpK21SZTFFaFdURHJra1BKR014?=
 =?utf-8?B?TkFZcUk0WVNzV2UvWHQ1dCtOSU5NNDBYcWYwUkxGRktMQXFPZjhZQVVBWWt4?=
 =?utf-8?B?OFhFaDFtbCtXTUVDZDlYbkZCVWFuY011b2tiUFp4N3FSRkZBK09TZGVQOS8x?=
 =?utf-8?B?NFZpdVBaN2pOTVgwK0U1d0JWY3E3THBqdFQ4YnFrUVRrakVXWXpQQ00raFlQ?=
 =?utf-8?B?WFBIMzBDTy9vRU5rZi9MdnJKRkFSSTBzeVRJbnJtNVNNWnluaDVzR21tTzFh?=
 =?utf-8?B?UkZpbGxEYVlnZExneU5qMmlHR0YrT3gxYXN3WWFrUzJOQ2hZeFlmS09RaEhm?=
 =?utf-8?B?RUZRYWl4Z1BGOEtuMzg5LzBvUGd5M0Z5TkhkRm5iLzYvNWt3TFlGUXJQSENs?=
 =?utf-8?B?YnhUVWJyempRMWdnaGNJcitKUTRXYVBtckVSNThyUE54d0MwOXJNL0pONlVT?=
 =?utf-8?B?SjJGd09vMGJDbUJpMnBMelhBeCt4TFpFVzBrQ2ZUMXhoUTNSSUhyRVhrUCtZ?=
 =?utf-8?B?c2x3ZWsyckZSSTNqNmtBN2J2KzIvbmE1NGxPTU0wY25pY2Z4MGJWTFYzODZX?=
 =?utf-8?B?R1BzYTVlSDB1QlpZTWY1MVlJZnczMHlVQVFUN3NFTnhkYmlMU2gxZER4M0to?=
 =?utf-8?B?bVNSMUp0clRKT1RMdzVJQW5DTVZlTmp2Q2dZY2NQdlNCbjdkVHplbStublhM?=
 =?utf-8?B?dGJBaXVuQnFmRC93eDJ0RWZ0bWJuU2oxRS8zdVpYRGh5endQa29nL0JjdmdZ?=
 =?utf-8?B?Tkd0SEpYMkVzSk1pVWk1M1Z6cEljZU4xV0RtVmJrcnhyeUVuVTdmVVFBNkxi?=
 =?utf-8?B?cmR3QzRvR1d5SWNUN1h2cVpBNWZDSHRQSi9YSm1iS0MzaFFXVmdWTi9WbzZJ?=
 =?utf-8?B?VEpQUXo3cTVCT1drek1tellKMTU1Uzh6VFpCN3ZQYUw4RmZoSnJvbVk2Unhm?=
 =?utf-8?B?WWdoVk92QTlhaFBaZ1JQSHhmQ1lacCsrQlBtcWxDTmdvVXZBQ2orcFYzbXp1?=
 =?utf-8?B?bDgrcVNYbGVhelNiOFhZNFduc1p2V3VzUVZ5ZzBwanFxNnZZK0Z5OWtuTUF6?=
 =?utf-8?B?TzZDd3JGNEw4a0RIdWRyWkN0WjIxSm45WXB5SC85YUNhdVd3K1QycjgwQ1c1?=
 =?utf-8?B?QjBQa1BURnhjZkZqUytDaFJ1SXBhK2d4M1hkUHZqdmt6bFEzU1lTZWNPY0ZY?=
 =?utf-8?B?MnorYzJZY1pvbjJZRTVEaXZhQ2daaDZoUnlmblhMUU82b2s3QVQwMW5OSWZY?=
 =?utf-8?B?UzRZOTZxcnZxVUh5MXdQdGY0by9yOGFoWmliT1pQMmtsemoxdWM3dTBpUzVk?=
 =?utf-8?B?Z0pmYkVreW9CQVdOdG9hSkNhQTh3dU5kWVVjSUlrVWY4ZXhSb0pveU5iN0Nu?=
 =?utf-8?B?blpoU3B1RGowaDY5bjcvYTJyK2xXVitGTjZ2ZUhENVdrZHNJRGpraGd4RW5o?=
 =?utf-8?B?ZHBoeHg3K2dHOUcrbksxeCtpVVFEZ3Q0cTVYanREZ3hUNWxPNFR4UGttclRH?=
 =?utf-8?B?MkFHbGxiY0hsZXNManI0UVhnVFVQWnZDZ1FTT2tudFZEbGpPRExwZzNycytS?=
 =?utf-8?B?ZkxmaklKbnUvcG1Bc0RVNXZ1ZmYyMTB3MVgzMFUrK29odE1ZZ0Z0b1V4SExF?=
 =?utf-8?B?N1E9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	wVKZvAWS6VAqtkEprj8hcy/mQP0BEJiGAmrZWzLCxRqvghREf6EKwjAU4oRxAlnuB4wuK7ngglQMDXOVK0erp300HUZlbZHJgLv9q/ZxiogWhDaXUjuQLNqPu1JblCbSL7OHMScpc6STGNmSYiXSgJLKyeloZ1YQs3GBhC4v9XdjDOJ2ZMtKH2arO/xCqm8lXouJ9vgQhgZsRRuZ9jd4MYNg8KvmqXiROu16ejn6YmGJq7pKYOS2FFq4W3oPQkFvsuu99T0jkkYmms1xtMwwjxVjvBOFyEVbNiFQM1D7XFPTXUFvgV/g6BMVjNNtb0gfbTC7Hdj01ojO2L0lbkOIrJ7UVJTH3/8ISTtLidyv/c69cM3aYb5Yb1AtgqprOtX4r022HTS/lOA+KT/2GgZZRCgf1BtBdc0M139a3l1N7kwefizpjtZkf4YWtHA1IFdADnQiwqZqvr8piGkfHY+D+U0s0SZZMJTU6m6Ir/L6mRje1ejcMLXL0a4VvqGcGifgT5BZcTf19FAW5wGIrLRWE4nNQ7xw1KKTfyp2t9SUyUxf563kisAlD4cren7YRx0hRJOwu0qDB6qX4zzc0o7duTR+0lweYaqwb539eBHsXPU=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9354f457-4c42-4984-89fd-08dc6531faf3
X-MS-Exchange-CrossTenant-AuthSource: MW5PR10MB5738.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Apr 2024 14:14:12.0259
 (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: WqVDcm1Ph4lAtv8QTkptq46+0ItGNzNOaQ7r4Y4Aai+ZXlOevJiO6SHzuWlzYZZ61sCkLzhwXHMWkhXHChnNenUy/Jka++lBh0qSeGhKWI0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR10MB7984
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-04-25_14,2024-04-25_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 suspectscore=0 bulkscore=0
 mlxlogscore=999 spamscore=0 malwarescore=0 mlxscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2404250104
X-Proofpoint-GUID: jOrcP_kuTHOtTNfwC70Nvm1bKQxx3ulR
X-Proofpoint-ORIG-GUID: jOrcP_kuTHOtTNfwC70Nvm1bKQxx3ulR

On 4/25/24 9:10AM, Jeongjun Park wrote:
> Matthew Wilcox wrote:
>> If that's the problem then the correct place to detect & reject this is
>> during mount, not at inode free time.
> 
> I fixed the patch as you said. If you patch in this way, the
> file system will not be affected by the vulnerability at all
> due to the code structure.
> 
> Thanks.
> 
> ---
>   fs/jfs/jfs_imap.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/jfs/jfs_imap.c b/fs/jfs/jfs_imap.c
> index 2ec35889ad24..ba0aa2f145cc 100644
> --- a/fs/jfs/jfs_imap.c
> +++ b/fs/jfs/jfs_imap.c
> @@ -290,7 +290,7 @@ int diSync(struct inode *ipimap)
>   int diRead(struct inode *ip)
>   {
>   	struct jfs_sb_info *sbi = JFS_SBI(ip->i_sb);
> -	int iagno, ino, extno, rc;
> +	int iagno, ino, extno, rc, agno;
>   	struct inode *ipimap;
>   	struct dinode *dp;
>   	struct iag *iagp;
> @@ -339,6 +339,9 @@ int diRead(struct inode *ip)
>   
>   	/* get the ag for the iag */
>   	agstart = le64_to_cpu(iagp->agstart);
> +	agno = BLKTOAG(agstart, JFS_SBI(ip->i_sb));
> +	if(agno >= MAXAG || agno < 0)
> +		return -EIO;

That's the right idea, but move the new code after the call to 
release_metapage().
>   
>   	release_metapage(mp);
>