From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out30-97.freemail.mail.aliyun.com (out30-97.freemail.mail.aliyun.com [115.124.30.97]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9AFFB481B4 for ; Mon, 9 Sep 2024 15:40:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=115.124.30.97 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725896421; cv=none; b=YFma7S8nwwxXdlKKv5gFwvkKpgEL6Uf5Wgxd8pgkC0IMK4fSmhiDITYQGShCgSWcDdPsXE01Yrv1GeK2t0x6jvcyzCgQPEH3WDaM0M8mJubN1HqzR8hhi64+wdv3eTVPDdH93W1jiLJnb4WuLZNCdEB89jOao4FqXuY/CMc/MiY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1725896421; c=relaxed/simple; bh=QbYcnWDQCG+6kXwBmBigkExYB0vsUVLpYXji+lR8T6Y=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=qmAD/g5Kmm1fm1OS8zpfXbNkU/Evlp1wwEipfa7xEtaAjp2bpAWsCBnLWKNJ9TLFZaoF5MwmFRa+4uNW2bTKd8sg5FuF5LxAvUJkAaYuF5NZOXazrKK3zYfDWVR6+DkThY31S4bXPVsVDq8lxoM9cQfSwR1CmbE2J6KUNIqniSY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com; spf=pass smtp.mailfrom=linux.alibaba.com; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b=Iqmqpzmm; arc=none smtp.client-ip=115.124.30.97 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.alibaba.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.alibaba.com header.i=@linux.alibaba.com header.b="Iqmqpzmm" DKIM-Signature:v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.alibaba.com; s=default; t=1725896416; h=Message-ID:Date:MIME-Version:Subject:To:From:Content-Type; bh=YIYQsQqU7SRbe/99jsN7MhA0jKoE0V/rgii7buJK1JY=; b=IqmqpzmmMIEN3ekoK7/Y/LpmtGn6M6nC3HSM3AuG+G6hf++f413aHFVYpd16DKeD2MdWdbcpGD9Gc179lyzvzlcpS3Vtq1n1Jewmw7nyKTaiX1Qvjvmj7rZ9eIF+EHtZ+yVQZDNe3wpRVA0PoVPbrgziKqbyGunuWnE3gzd3CPM= Received: from 192.168.2.29(mailfrom:hsiangkao@linux.alibaba.com fp:SMTPD_---0WEgzJpI_1725896414) by smtp.aliyun-inc.com; Mon, 09 Sep 2024 23:40:15 +0800 Message-ID: Date: Mon, 9 Sep 2024 23:40:14 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] erofs: fix incorrect symlink detection in fast symlink To: Colin Walters , linux-erofs@lists.ozlabs.org Cc: LKML References: <20240909022811.1105052-1-hsiangkao@linux.alibaba.com> <20240909031911.1174718-1-hsiangkao@linux.alibaba.com> <25f0356d-d949-483c-8e59-ddc9cace61f6@linux.alibaba.com> <21ddadb7-407d-48b6-9c1b-845ead2eefb4@app.fastmail.com> <91310d4c-98d5-4a8b-b3db-2043d4a3d533@app.fastmail.com> From: Gao Xiang In-Reply-To: <91310d4c-98d5-4a8b-b3db-2043d4a3d533@app.fastmail.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 2024/9/9 22:46, Colin Walters wrote: > > > On Mon, Sep 9, 2024, at 10:14 AM, Gao Xiang wrote: >> >> Not quite sure about hard limitation in EROFS >> runtime, we could define >> >> #define EROFS_SYMLINK_MAXLEN 4096 > > Not sure that a new define is needed versus just reusing PATH_MAX, but that's obviously just a style thing that's much more your call than mine. > >> But since symlink i_size > 4096 only due to crafted >> images (and not generated by mkfs) and not crash, so >> either way (to check or not check in kernel) is okay >> to me. > > Yes, but my understanding was that EROFS (in contrast to other kernel read-write filesystems which are more complicated) was aiming to be robust against potentially malicious images. Just my personal opinion, my understanding of rubustness is stability and security. But whether to check or not check this, it doesn't crash the kernel or deadlock or livelock, so IMHO, it's already rubustness. Actually, I think EROFS for i_size > PAGE_SIZE, it's an undefined or reserved behavior for now (just like CPU reserved bits or don't care bits), just Linux implementation treats it with PAGE_SIZE-1 trailing '\0', but using erofs dump tool you could still dump large symlinks. Since PATH_MAX is a system-defined constant too, currently Linux PATH_MAX is 4096, but how about other OSes? I've seen some `PATH_MAX 8192` reference but I'm not sure which OS uses this setting. But I think it's a filesystem on-disk limitation, but if i_size exceeds that, we return -EOPNOTSUPP or -EFSCORRUPTED? For this symlink case, I tend to return -EFSCORRUPTED but for other similar but complex cases, it could be hard to decide. Leaving them as undefined behaviors are also an option as long as the behavior is secure. > > Crafted/malicious images aside, there's also the IMO obvious angle here that we should avoid crashes or worse out-of-bound read/write if there happens to be *accidental* on-disk/memory corruption and having high bit(s) flip in a symlink inode size seems like an easy one to handle. Skimming the XFS code for example it looks like it's pretty robust in this area. Yes, for this case it's much simple and easy so that's fine, but I think for some other cases, leaving some undefined or reserved behaviors are also good for later extendability (again, like CPU register design.) as long as it doesn't cause security issues. Thanks, Gao Xiang