| Message ID | 1582718512-28923-1-git-send-email-macpaul.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v4] usb: gadget: f_fs: try to fix AIO issue under ARM 64 bit TAGGED mode | expand |
On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin wrote: > This issue was found when adbd trying to open functionfs with AIO mode. > Usually, we need to set "setprop sys.usb.ffs.aio_compat 0" to enable > adbd with AIO mode on Android. > > When adbd is opening functionfs, it will try to read 24 bytes at the > first read I/O control. If this reading has been failed, adbd will > try to send FUNCTIONFS_CLEAR_HALT to functionfs. When adbd is in AIO > mode, functionfs will be acted with asyncronized I/O path. After the > successful read transfer has been completed by gadget hardware, the > following series of functions will be called. > ffs_epfile_async_io_complete() -> ffs_user_copy_worker() -> > copy_to_iter() -> _copy_to_iter() -> copyout() -> > iterate_and_advance() -> iterate_iovec() > > Adding debug trace to these functions, it has been found that in > copyout(), access_ok() will check if the user space address is valid > to write. However if CONFIG_ARM64_TAGGED_ADDR_ABI is enabled, adbd > always passes user space address start with "0x3C" to gadget's AIO > blocks. This tagged address will cause access_ok() check always fail. > Which causes later calculation in iterate_iovec() turn zero. > Copyout() won't copy data to user space since the length to be copied > "v.iov_len" will be zero. Finally leads ffs_copy_to_iter() always return > -EFAULT, causes adbd cannot open functionfs and send > FUNCTIONFS_CLEAR_HALT. > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > Cc: Peter Chen <peter.chen@nxp.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Miles Chen <miles.chen@mediatek.com> > --- > Changes for v4: > - Abandon solution v3 by adding "TIF_TAGGED_ADDR" flag to gadget driver. > According to Catalin's suggestion, change the solution by untagging > user space address passed by AIO in gadget driver. Well, this was suggested in case you have a strong reason not to do the untagging in adbd. As I said, tagged pointers in user space were supported long before we introduced CONFIG_ARM64_TAGGED_ADDR_ABI. How did adb cope with such tagged pointers before? It was not supposed to pass them to the kernel. > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index ce1d023..192935f 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) > > static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) > { > - ssize_t ret = copy_to_iter(data, data_len, iter); > + ssize_t ret; > + > +#if defined(CONFIG_ARM64) > + /* > + * Replace tagged address passed by user space application before > + * copying. > + */ > + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > + (iter->type == ITER_IOVEC)) { > + *(unsigned long *)&iter->iov->iov_base = > + (unsigned long)untagged_addr(iter->iov->iov_base); > + } > +#endif > + ret = copy_to_iter(data, data_len, iter); Here you should probably drop all the #ifdefs and IS_ENABLED checks since untagged_addr() is defined globally as a no-op (and overridden by arm64 and sparc). Please don't send another patch until we understand (a) whether this is a user-space problem to fix or (b) if we fix it in the kernel, is this the only/right place? If we settle for the in-kernel untagging, do we explicitly untag the addresses in such kernel threads or we default to TIF_TAGGED_ADDR for all kernel threads, in case they ever call use_mm() (or we could even hook something in use_mm() to set this TIF flag temporarily). Looking for feedback from the Android folk and a better analysis of the possible solution.
On Thu, 2020-02-27 at 09:55 +0000, Catalin Marinas wrote: > On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin wrote: > > This issue was found when adbd trying to open functionfs with AIO mode. > > Usually, we need to set "setprop sys.usb.ffs.aio_compat 0" to enable > > adbd with AIO mode on Android. > > > > When adbd is opening functionfs, it will try to read 24 bytes at the > > first read I/O control. If this reading has been failed, adbd will > > try to send FUNCTIONFS_CLEAR_HALT to functionfs. When adbd is in AIO > > mode, functionfs will be acted with asyncronized I/O path. After the > > successful read transfer has been completed by gadget hardware, the > > following series of functions will be called. > > ffs_epfile_async_io_complete() -> ffs_user_copy_worker() -> > > copy_to_iter() -> _copy_to_iter() -> copyout() -> > > iterate_and_advance() -> iterate_iovec() > > > > Adding debug trace to these functions, it has been found that in > > copyout(), access_ok() will check if the user space address is valid > > to write. However if CONFIG_ARM64_TAGGED_ADDR_ABI is enabled, adbd > > always passes user space address start with "0x3C" to gadget's AIO > > blocks. This tagged address will cause access_ok() check always fail. > > Which causes later calculation in iterate_iovec() turn zero. > > Copyout() won't copy data to user space since the length to be copied > > "v.iov_len" will be zero. Finally leads ffs_copy_to_iter() always return > > -EFAULT, causes adbd cannot open functionfs and send > > FUNCTIONFS_CLEAR_HALT. > > > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > > Cc: Peter Chen <peter.chen@nxp.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Miles Chen <miles.chen@mediatek.com> > > --- > > Changes for v4: > > - Abandon solution v3 by adding "TIF_TAGGED_ADDR" flag to gadget driver. > > According to Catalin's suggestion, change the solution by untagging > > user space address passed by AIO in gadget driver. > > Well, this was suggested in case you have a strong reason not to do the > untagging in adbd. As I said, tagged pointers in user space were > supported long before we introduced CONFIG_ARM64_TAGGED_ADDR_ABI. How > did adb cope with such tagged pointers before? It was not supposed to > pass them to the kernel. Thank for your explanation. Since adbd was developed by Google and we can only suggest (like, file an issue) to them. Here provides a temporary solution for other developer to solve there needs in a short period. Yes, I understood not supposed to pass those tagged pointers to kernel and will also explain this to Google adbd owners. > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > > index ce1d023..192935f 100644 > > --- a/drivers/usb/gadget/function/f_fs.c > > +++ b/drivers/usb/gadget/function/f_fs.c > > @@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) > > > > static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) > > { > > - ssize_t ret = copy_to_iter(data, data_len, iter); > > + ssize_t ret; > > + > > +#if defined(CONFIG_ARM64) > > + /* > > + * Replace tagged address passed by user space application before > > + * copying. > > + */ > > + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > > + (iter->type == ITER_IOVEC)) { > > + *(unsigned long *)&iter->iov->iov_base = > > + (unsigned long)untagged_addr(iter->iov->iov_base); > > + } > > +#endif > > + ret = copy_to_iter(data, data_len, iter); > > Here you should probably drop all the #ifdefs and IS_ENABLED checks > since untagged_addr() is defined globally as a no-op (and overridden by > arm64 and sparc). > > Please don't send another patch until we understand (a) whether this is > a user-space problem to fix or (b) if we fix it in the kernel, is this > the only/right place? If we settle for the in-kernel untagging, do we > explicitly untag the addresses in such kernel threads or we default to > TIF_TAGGED_ADDR for all kernel threads, in case they ever call use_mm() > (or we could even hook something in use_mm() to set this TIF flag > temporarily). > > Looking for feedback from the Android folk and a better analysis of the > possible solution. > If we have any further update about this user space issue, I'll update the solution to this thread for other developers who need to solve this issue. Thanks! Macpaul Lin
On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin wrote: > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > index ce1d023..192935f 100644 > --- a/drivers/usb/gadget/function/f_fs.c > +++ b/drivers/usb/gadget/function/f_fs.c > @@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) > > static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) > { > - ssize_t ret = copy_to_iter(data, data_len, iter); > + ssize_t ret; > + > +#if defined(CONFIG_ARM64) > + /* > + * Replace tagged address passed by user space application before > + * copying. > + */ > + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > + (iter->type == ITER_IOVEC)) { > + *(unsigned long *)&iter->iov->iov_base = > + (unsigned long)untagged_addr(iter->iov->iov_base); > + } > +#endif > + ret = copy_to_iter(data, data_len, iter); > if (likely(ret == data_len)) > return ret; I had forgotten that we discussed a similar case already a few months ago (thanks to Evgenii for pointing out). Do you have this commit applied to your tree: df325e05a682 ("arm64: Validate tagged addresses in access_ok() called from kernel threads")?
On Fri, 2020-02-28 at 16:48 +0000, Catalin Marinas wrote: > On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin wrote: > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > > index ce1d023..192935f 100644 > > --- a/drivers/usb/gadget/function/f_fs.c > > +++ b/drivers/usb/gadget/function/f_fs.c > > @@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) > > > > static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) > > { > > - ssize_t ret = copy_to_iter(data, data_len, iter); > > + ssize_t ret; > > + > > +#if defined(CONFIG_ARM64) > > + /* > > + * Replace tagged address passed by user space application before > > + * copying. > > + */ > > + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > > + (iter->type == ITER_IOVEC)) { > > + *(unsigned long *)&iter->iov->iov_base = > > + (unsigned long)untagged_addr(iter->iov->iov_base); > > + } > > +#endif > > + ret = copy_to_iter(data, data_len, iter); > > if (likely(ret == data_len)) > > return ret; > > I had forgotten that we discussed a similar case already a few months > ago (thanks to Evgenii for pointing out). Do you have this commit > applied to your tree: df325e05a682 ("arm64: Validate tagged addresses in > access_ok() called from kernel threads")? > Yes! We have that patch. I've also got Google's reply about referencing this patch in android kernel tree. https://android-review.googlesource.com/c/kernel/common/+/1186615 However, during my debugging process, I've dumped specific length (e.g., 24 bytes for the first request) AIO request buffer address both in adbd and in __range_ok(). Then I've found __range_ok() still always return false on address begin with "0x3c". Since untagged_addr() already called in __range_ok(), to set "TIF_TAGGED_ADDR" with adbd's user space buffer should be the possible solution. Hence I've send the v3 patch. Anyway, I've found that to disable TAGGED address in adbd is possible by this way and will report to Google and see how they think. diff --git a/adb/daemon/main.cpp b/adb/daemon/main.cpp index 9e02e89ab..b2f6f8e3f 100644 --- a/adb/daemon/main.cpp +++ b/adb/daemon/main.cpp @@ -317,6 +317,8 @@ int main(int argc, char** argv) { mallopt(M_DECAY_TIME, 1); #endif + prctl(PR_SET_TAGGED_ADDR_CTRL, ~PR_TAGGED_ADDR_ENABLE, 0, 0, 0); + while (true) { static struct option opts[] = { {"root_seclabel", required_argument, nullptr, 's'}, Many thanks! Macpaul Lin
On Sun, Mar 01, 2020 at 11:20:43AM +0800, Macpaul Lin wrote: > On Fri, 2020-02-28 at 16:48 +0000, Catalin Marinas wrote: > > On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin wrote: > > > diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c > > > index ce1d023..192935f 100644 > > > --- a/drivers/usb/gadget/function/f_fs.c > > > +++ b/drivers/usb/gadget/function/f_fs.c > > > @@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) > > > > > > static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) > > > { > > > - ssize_t ret = copy_to_iter(data, data_len, iter); > > > + ssize_t ret; > > > + > > > +#if defined(CONFIG_ARM64) > > > + /* > > > + * Replace tagged address passed by user space application before > > > + * copying. > > > + */ > > > + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > > > + (iter->type == ITER_IOVEC)) { > > > + *(unsigned long *)&iter->iov->iov_base = > > > + (unsigned long)untagged_addr(iter->iov->iov_base); > > > + } > > > +#endif > > > + ret = copy_to_iter(data, data_len, iter); > > > if (likely(ret == data_len)) > > > return ret; > > > > I had forgotten that we discussed a similar case already a few months > > ago (thanks to Evgenii for pointing out). Do you have this commit > > applied to your tree: df325e05a682 ("arm64: Validate tagged addresses in > > access_ok() called from kernel threads")? > > > > Yes! We have that patch. I've also got Google's reply about referencing > this patch in android kernel tree. > https://android-review.googlesource.com/c/kernel/common/+/1186615 > > However, during my debugging process, I've dumped specific length (e.g., > 24 bytes for the first request) AIO request buffer address both in adbd > and in __range_ok(). Then I've found __range_ok() still always return > false on address begin with "0x3c". Since untagged_addr() already called > in __range_ok(), to set "TIF_TAGGED_ADDR" with adbd's user space buffer > should be the possible solution. Hence I've send the v3 patch. ffs_copy_to_iter() is called from a workqueue (ffs_user_copy_worker()). That's still in a kernel thread context but it doesn't have PF_KTHREAD set, hence __range_ok() rejects the tagged address. Can you try the diff below: diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 32fc8061aa76..2803143cad1f 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -68,7 +68,8 @@ static inline unsigned long __range_ok(const void __user *addr, unsigned long si * the user address before checking. */ if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && - (current->flags & PF_KTHREAD || test_thread_flag(TIF_TAGGED_ADDR))) + (current->flags & (PF_KTHREAD | PF_WQ_WORKER) || + test_thread_flag(TIF_TAGGED_ADDR))) addr = untagged_addr(addr); __chk_user_ptr(addr); -
On Tue, 2020-03-03 at 11:19 -0800, Evgenii Stepanov wrote: > I'm a bit surprised that this is necessary, given that the earlier > patch that added the (current->flags & PF_KTHREAD) condition was in > response to this exact problem, and I know for sure that it helped. > This was the stack trace for the call to __range_ok in that case: > [ 12.886765] c1 271 _copy_to_iter+0xb8/0x5c0 > > [ 12.891421] c1 271 ffs_user_copy_worker+0xec/0x24c > [ 12.896699] c1 271 process_one_work+0x264/0x450 > [ 12.901713] c1 271 worker_thread+0x250/0x484 > [ 12.906454] c1 271 kthread+0x11c/0x12c > [ 12.910664] c1 271 ret_from_fork+0x10/0x18 > It would be great to know what changed to require the updated > condition. > Adding a prctl call to adb is unlikely to help, because it would not > stop tagged address generation in malloc. Sorry for late reply, after carefully check the kerenl update status in Mediatek's branch. I've found we got this patch (df325e05a682 ("arm64: Validate tagged addresses in access_ok() called from kernel threads")) updated into internal Mediatek's working tree around Feb 23 or 24. However, I'm not sure why that patch cannot work in my own working tree at that time. I've indeed dumped user space address and checked the return result in access_ok() and found it was not worked. In these days I've clean up all my working space and re-test this patch, I've found to check PF_KTHREAD and TIF_TAGGED_ADDR was enough to solve this problem. Sorry for bothering I'm not sure what causes that fail in previous environment. Moreover, I've tested PF_WQ_WORKER case, if we replaced test flag PF_KTHREAD by PF_WQ_WORKER, AIO will still be worked, too. Both code A. if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && (current->flags & PF_KTHREAD || test_thread_flag(TIF_TAGGED_ADDR))) or B. if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && (current->flags & PF_WQ_WORKER || test_thread_flag(TIF_TAGGED_ADDR))) are worked for this issue. > On Mon, Mar 2, 2020 at 8:19 AM Catalin Marinas > <catalin.marinas@arm.com> wrote: > > On Sun, Mar 01, 2020 at 11:20:43AM +0800, Macpaul Lin wrote: > > On Fri, 2020-02-28 at 16:48 +0000, Catalin Marinas wrote: > > > On Wed, Feb 26, 2020 at 08:01:52PM +0800, Macpaul Lin > wrote: > > > > diff --git a/drivers/usb/gadget/function/f_fs.c > b/drivers/usb/gadget/function/f_fs.c > > > > index ce1d023..192935f 100644 > > > > --- a/drivers/usb/gadget/function/f_fs.c > > > > +++ b/drivers/usb/gadget/function/f_fs.c > > > > @@ -715,7 +715,20 @@ static void > ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request > *req) > > > > > > > > static ssize_t ffs_copy_to_iter(void *data, int > data_len, struct iov_iter *iter) > > > > { > > > > - ssize_t ret = copy_to_iter(data, data_len, iter); > > > > + ssize_t ret; > > > > + > > > > +#if defined(CONFIG_ARM64) > > > > + /* > > > > + * Replace tagged address passed by user space > application before > > > > + * copying. > > > > + */ > > > > + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > > > > + (iter->type == ITER_IOVEC)) { > > > > + *(unsigned long *)&iter->iov->iov_base = > > > > + (unsigned > long)untagged_addr(iter->iov->iov_base); > > > > + } > > > > +#endif > > > > + ret = copy_to_iter(data, data_len, iter); > > > > if (likely(ret == data_len)) > > > > return ret; > > > > > > I had forgotten that we discussed a similar case already a > few months > > > ago (thanks to Evgenii for pointing out). Do you have this > commit > > > applied to your tree: df325e05a682 ("arm64: Validate > tagged addresses in > > > access_ok() called from kernel threads")? > > > > > > > Yes! We have that patch. I've also got Google's reply about > referencing > > this patch in android kernel tree. > > > https://android-review.googlesource.com/c/kernel/common/+/1186615 > > > > However, during my debugging process, I've dumped specific > length (e.g., > > 24 bytes for the first request) AIO request buffer address > both in adbd > > and in __range_ok(). Then I've found __range_ok() still > always return > > false on address begin with "0x3c". Since untagged_addr() > already called > > in __range_ok(), to set "TIF_TAGGED_ADDR" with adbd's user > space buffer > > should be the possible solution. Hence I've send the v3 > patch. > > ffs_copy_to_iter() is called from a workqueue > (ffs_user_copy_worker()). > That's still in a kernel thread context but it doesn't have > PF_KTHREAD > set, hence __range_ok() rejects the tagged address. Can you > try the diff > below: > > diff --git a/arch/arm64/include/asm/uaccess.h > b/arch/arm64/include/asm/uaccess.h > index 32fc8061aa76..2803143cad1f 100644 > --- a/arch/arm64/include/asm/uaccess.h > +++ b/arch/arm64/include/asm/uaccess.h > @@ -68,7 +68,8 @@ static inline unsigned long __range_ok(const > void __user *addr, unsigned long si > * the user address before checking. > */ > if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && > - (current->flags & PF_KTHREAD || > test_thread_flag(TIF_TAGGED_ADDR))) > + (current->flags & (PF_KTHREAD | PF_WQ_WORKER) || > + test_thread_flag(TIF_TAGGED_ADDR))) > addr = untagged_addr(addr); > > __chk_user_ptr(addr); > - Many thanks to Catalin and Evgenii. Regards, Macpaul Lin
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c index ce1d023..192935f 100644 --- a/drivers/usb/gadget/function/f_fs.c +++ b/drivers/usb/gadget/function/f_fs.c @@ -715,7 +715,20 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) { - ssize_t ret = copy_to_iter(data, data_len, iter); + ssize_t ret; + +#if defined(CONFIG_ARM64) + /* + * Replace tagged address passed by user space application before + * copying. + */ + if (IS_ENABLED(CONFIG_ARM64_TAGGED_ADDR_ABI) && + (iter->type == ITER_IOVEC)) { + *(unsigned long *)&iter->iov->iov_base = + (unsigned long)untagged_addr(iter->iov->iov_base); + } +#endif + ret = copy_to_iter(data, data_len, iter); if (likely(ret == data_len)) return ret;
This issue was found when adbd trying to open functionfs with AIO mode. Usually, we need to set "setprop sys.usb.ffs.aio_compat 0" to enable adbd with AIO mode on Android. When adbd is opening functionfs, it will try to read 24 bytes at the first read I/O control. If this reading has been failed, adbd will try to send FUNCTIONFS_CLEAR_HALT to functionfs. When adbd is in AIO mode, functionfs will be acted with asyncronized I/O path. After the successful read transfer has been completed by gadget hardware, the following series of functions will be called. ffs_epfile_async_io_complete() -> ffs_user_copy_worker() -> copy_to_iter() -> _copy_to_iter() -> copyout() -> iterate_and_advance() -> iterate_iovec() Adding debug trace to these functions, it has been found that in copyout(), access_ok() will check if the user space address is valid to write. However if CONFIG_ARM64_TAGGED_ADDR_ABI is enabled, adbd always passes user space address start with "0x3C" to gadget's AIO blocks. This tagged address will cause access_ok() check always fail. Which causes later calculation in iterate_iovec() turn zero. Copyout() won't copy data to user space since the length to be copied "v.iov_len" will be zero. Finally leads ffs_copy_to_iter() always return -EFAULT, causes adbd cannot open functionfs and send FUNCTIONFS_CLEAR_HALT. Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> Cc: Peter Chen <peter.chen@nxp.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Miles Chen <miles.chen@mediatek.com> --- Changes for v4: - Abandon solution v3 by adding "TIF_TAGGED_ADDR" flag to gadget driver. According to Catalin's suggestion, change the solution by untagging user space address passed by AIO in gadget driver. Changes for v3: - Fix misspelling in commit message. Thanks for Peter's review. Changes for v2: - Fix build error for 32-bit load. An #if defined(CONFIG_ARM64) still need for avoiding undeclared defines. drivers/usb/gadget/function/f_fs.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)