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: <12be770a-ca1c-7231-ff98-661211f2d8ec@windriver.com>
Date:   Tue, 16 Aug 2022 09:00:33 +0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.11.0
Subject: Re: [PATCH] asm/sections: fix the determination of the end of the
 memory region
Content-Language: en-US
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     Arnd Bergmann <arnd@arndb.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        Thierry Reding <treding@nvidia.com>,
        linux-arch@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20220811063105.2553381-1-quanyang.wang@windriver.com>
 <CAMj1kXHxeGjbYXhXpgRBM60Hobo+0K2JkjEtm+7kzDOYobSmNA@mail.gmail.com>
From:   Quanyang Wang <quanyang.wang@windriver.com>
In-Reply-To: <CAMj1kXHxeGjbYXhXpgRBM60Hobo+0K2JkjEtm+7kzDOYobSmNA@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Zk5RODg0L3Q2RmlaWVRoTmNXakdFV2pldWREZWhCU3ZPNkhGR2VpenQ2WENP?=
 =?utf-8?B?YUJPUGxaMEk3TENUbHFpVHJJOW5Qd0ZBWVF4ZWtONTNmYkc3dVEyZ2hUOWZM?=
 =?utf-8?B?dDl5THFRc1N4Q3pKRFdXbXdvVFFUQ29rcWxvZ0ZtZjZiVUdpMXdsQnp1ci9H?=
 =?utf-8?B?RXJWZkgrSjhkS2xhU0F5Y1o1WFVXR3dwaU90Z2JYSUR5R2hqMzFjb0pFZm9m?=
 =?utf-8?B?cXY1VUNvL1N3ekY5ZFRHcmVqc3pWZzA5RVRCUzZrd0trS0xpUm5wbW1YdjlP?=
 =?utf-8?B?WS9mcmRoRWVVbVlQTndmWHQwRGtTL3Vqb2t4MHNrRWNzOU44Tzg0T3dGSEEx?=
 =?utf-8?B?QXd5NWNIeFNYaUVVajVhU2w2dXFFRU9EbU1QKzN5KzFBbFBkUCttbU1pc3Rj?=
 =?utf-8?B?QWpzdUpvVEFPaXVjOVcwMEM2WUYvdlFYV1hDNVMxakhCNmg2clVEL2NJRFZo?=
 =?utf-8?B?R3ZTQ2xvTjhmK1VLV2RSMkpIN1RpTjJxaDlRTCtNY0VEMkFHclBGVUNuZHJi?=
 =?utf-8?B?V0J6WC95SHFKc0J5SklPOFdoVkVNMTZrOGJqby92UEljWXBaRmZ4emcyM3dS?=
 =?utf-8?B?cnBGa3FiKytvWnY1SnZ6VXJTNXRJM3lUNGFDWm90cEhWWUVEdEtPKzdqR1JR?=
 =?utf-8?B?SGVTVUFFbWVyT0xLUHJLMld4S0w4ZjZremdpbUh0QWpRLy9nSjNOK3hDTHJ4?=
 =?utf-8?B?KzdCd1VZQ3NiRzN3dG9QSEN6VTdWbEN4QmdIZFBqdFFabUhaUEdIR3BDekdi?=
 =?utf-8?B?Vm9valFYV3FOZ2RQYnVjb1lJUjZ1bDZ1WGJWdk01Y0hockZpVVFldE0xSm1D?=
 =?utf-8?B?Z0ZWeGdVWWRjYW1aRDU1TU1Bc1FNWFdyN2tuRWZYWTBtcmp2cDRHYnpwajUx?=
 =?utf-8?B?UGNDc1FnRzlJWm9BZjlkejFURWViRFdnL0s0WUlUTjZnWDZvVG9FWEFJaytp?=
 =?utf-8?B?K3hjNDVLRHJLSWZpd0JYUXlzSk9ZN1kzRHR6TWRPeWxxOHlYeHo2UHlFc3Br?=
 =?utf-8?B?WXh2WGZRbWVlNnhkMzMzYjlDbWpSZlhBdTdmejNtSTZMaUczcllUVDZBc1d2?=
 =?utf-8?B?TmZzU3VYemNBV0VFQ2FqU3BLeEQ3RVdUOW94elhyLzdLTmgzUmRIWHhqZzJr?=
 =?utf-8?B?Si9xaHNvWWZ6bWkxNXhyeG1JbmlibXQ4a2w5am5rWTFtWlZsNDQwVHE1OXRC?=
 =?utf-8?B?N29rWDFqSHJ1TUVibTdmOUt2bnVQVG55Y0ZQV0VpZTRGNFRPVzd4U0t1c2lp?=
 =?utf-8?B?QzA2UnU3b3Voc2lJRDBKZzB3eDJGMUVKbjY1dVdnSlRlSkdKcmdSbzBQR1Rp?=
 =?utf-8?B?OXUwSXhXc3NCTGo0aHZZZVgvNlFSbDA1bERRQUI4VmhBMkZIS09kUUp2SWZu?=
 =?utf-8?B?T3U1V0JUSWthY3hCd1AzVG5nYVp6UnBUcHZ1WmNIUld6TnJsT2NDeXRhM3p3?=
 =?utf-8?B?NEVJT3ViWWdPdnA0QjFMdzJyY0hOWlcreXdUWTBYZ0tiMlV6RzJpcDkrVXlw?=
 =?utf-8?B?bWtISFNHWU4yaDNEM0NmWEcwdld1VFZaKzZiNmpLUFFjNTNmbTBXZndWVXox?=
 =?utf-8?B?dHkzTXpjNnJVVlJuVTA4ZVkvbFRseXh1UG95WDdhRzJ4YXdNdS94RjVVTUkw?=
 =?utf-8?B?aXhWazRqNGlCRy9razVkS3RtL3hIRnFUOXV1NUhDWisrMTZnR3Y1cFZBOWdI?=
 =?utf-8?B?bUY4MkxOTTRmWm5zZWdaa1VnVndQVnJyZDFmZ2ZpSFZNbjBFY3F3UUFzTlF3?=
 =?utf-8?B?T3pSbkI4TEF4RGM4WWNFNC9SeHZYY0hocTJidUU3MCtnMWt0dXNnMjNFQWlS?=
 =?utf-8?B?SHBtdVlNRnp2a3UrVnhETTJCbEE1UW9nSktibWNjSjdpNnFNTTk5dGF2d2Y0?=
 =?utf-8?B?ZWx5YTA4VlRaUUxyMXY4RFlzS3htRWVFQmhOejlCdjJ3cndTckIvWGFKbmNq?=
 =?utf-8?B?cG5ldVdrV1hUYjhQWmZEaTd2ckRjbU9ScGZSOGNybUFLaG5zVlQ5a3Fod0dn?=
 =?utf-8?B?RUFqTTcwalR3bFM3NWg4ZFpXZE05ZGl6VVU0WDJFU3BqdjVidVJsU3A1dHUy?=
 =?utf-8?B?ZTNwVTg3WmIvaC96bjk1NzdpcGdPWlJ3cDRRUW1UWFM1Ni9GbDhDQWFCSTBz?=
 =?utf-8?B?bWdKRmtmeUFLWnUwMVErQ2plUDRnaVVlUzdHeU92VGJuL1dlMm5BRkx0cEVN?=
 =?utf-8?B?bnc9PQ==?=
X-OriginatorOrg: windriver.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 75b567db-f690-4077-783b-08da7f22c229
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR11MB5630.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Aug 2022 01:00:49.2902
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 8ddb2873-a1ad-4a18-ae4e-4644631433be
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: E0C+H/xuu2MhOQsFGoAMOsRroHrh54iLRBhqj2jXuDLDRTOgxsRFm+BJIP9aD4fekGKA9+v1fv0VesE3Nki0ZdHEqwfqTfge1IjOsRORN+A=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY5PR11MB4242
X-Proofpoint-ORIG-GUID: gR4Xk8966Vh1BYnDEO_nXzxJ2R7H4xqd
X-Proofpoint-GUID: gR4Xk8966Vh1BYnDEO_nXzxJ2R7H4xqd
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.883,Hydra:6.0.517,FMLib:17.11.122.1
 definitions=2022-08-15_08,2022-08-15_01,2022-06-22_01
X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501
 mlxlogscore=946 lowpriorityscore=0 malwarescore=0 mlxscore=0 phishscore=0
 spamscore=0 adultscore=0 bulkscore=0 suspectscore=0 impostorscore=0
 clxscore=1011 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2207270000 definitions=main-2208160002
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,
        SPF_HELO_NONE,SPF_PASS,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

Hi Ard,

On 8/11/22 16:16, Ard Biesheuvel wrote:
> On Thu, 11 Aug 2022 at 08:31, <quanyang.wang@windriver.com> wrote:
>>
>> From: Quanyang Wang <quanyang.wang@windriver.com>
>>
>> If using "vend >= begin" to judge if two memory regions intersects, vend
>> should be the end of the memory region, so it should be "virt + size -1"
>> instead of "virt + size".
>> The wrong determination of the end triggers the misreporting as below when
>> the dma debug function "check_for_illegal_area" calls memory_intersects to
>> check if the dma region intersects with stext region.
>>
>> Calltrace (stext is at 0x80100000):
>>   WARNING: CPU: 0 PID: 77 at kernel/dma/debug.c:1073 check_for_illegal_area+0x130/0x168
>>   DMA-API: chipidea-usb2 e0002000.usb: device driver maps memory from kernel text or rodata [addr=800f0000] [len=65536]
>>   Modules linked in:
>>   CPU: 1 PID: 77 Comm: usb-storage Not tainted 5.19.0-yocto-standard #5
>>   Hardware name: Xilinx Zynq Platform
>>    unwind_backtrace from show_stack+0x18/0x1c
>>    show_stack from dump_stack_lvl+0x58/0x70
>>    dump_stack_lvl from __warn+0xb0/0x198
>>    __warn from warn_slowpath_fmt+0x80/0xb4
>>    warn_slowpath_fmt from check_for_illegal_area+0x130/0x168
>>    check_for_illegal_area from debug_dma_map_sg+0x94/0x368
>>    debug_dma_map_sg from __dma_map_sg_attrs+0x114/0x128
>>    __dma_map_sg_attrs from dma_map_sg_attrs+0x18/0x24
>>    dma_map_sg_attrs from usb_hcd_map_urb_for_dma+0x250/0x3b4
>>    usb_hcd_map_urb_for_dma from usb_hcd_submit_urb+0x194/0x214
>>    usb_hcd_submit_urb from usb_sg_wait+0xa4/0x118
>>    usb_sg_wait from usb_stor_bulk_transfer_sglist+0xa0/0xec
>>    usb_stor_bulk_transfer_sglist from usb_stor_bulk_srb+0x38/0x70
>>    usb_stor_bulk_srb from usb_stor_Bulk_transport+0x150/0x360
>>    usb_stor_Bulk_transport from usb_stor_invoke_transport+0x38/0x440
>>    usb_stor_invoke_transport from usb_stor_control_thread+0x1e0/0x238
>>    usb_stor_control_thread from kthread+0xf8/0x104
>>    kthread from ret_from_fork+0x14/0x2c
>>
>> Fixes: 979559362516 ("asm/sections: add helpers to check for section data")
>> Signed-off-by: Quanyang Wang <quanyang.wang@windriver.com>
>> ---
>>   include/asm-generic/sections.h | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>> index d0f7bdd2fdf2..f7171b4f5bfd 100644
>> --- a/include/asm-generic/sections.h
>> +++ b/include/asm-generic/sections.h
>> @@ -108,7 +108,7 @@ static inline bool memory_contains(void *begin, void *end, void *virt,
>>   static inline bool memory_intersects(void *begin, void *end, void *virt,
>>                                       size_t size)
>>   {
>> -       void *vend = virt + size;
>> +       void *vend = virt + size - 1;
>>
>>          return (virt >= begin && virt < end) || (vend >= begin && vend < end);
> 
> This test looks flawed to me for another reason as well: it only
> checks whether the start /or/ the end of (virt, virt+size) falls
> inside the area, so if the area is covered completely (in which case
> the intersection of the two will be equal to the area), this will
> return false erroneously.
Yes, the test lacks some checks. I will send a V2 patch to fix it.
Thanks,
Quanyang