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;
Date:   Thu, 14 Apr 2022 11:21:15 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Oliver Neukum <oneukum@suse.com>
Cc:     David Kahurani <k.kahurani@gmail.com>, netdev@vger.kernel.org,
        syzbot <syzbot+d3dbdf31fbe9d8f5f311@syzkaller.appspotmail.com>,
        davem@davemloft.net, jgg@ziepe.ca, kuba@kernel.org,
        linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
        Phillip Potter <phil@philpotter.co.uk>,
        syzkaller-bugs@googlegroups.com, arnd@arndb.de,
        Pavel Skripkin <paskripkin@gmail.com>
Subject: Re: [PATCH] net: ax88179: add proper error handling of usb read
 errors
Message-ID: <20220414082115.GA12805@kadam>
References: <20220404151036.265901-1-k.kahurani@gmail.com>
 <20220404153151.GF3293@kadam>
 <CAAZOf25i_mLO9igOY5wiUaxLOsxMt3jrvytSm1wm95R-bdKysA@mail.gmail.com>
 <20220413153249.GZ12805@kadam>
 <523330e4-cbd7-62a6-9368-417534ddb0b6@suse.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <523330e4-cbd7-62a6-9368-417534ddb0b6@suse.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?AnWNq/gSEbOzyT+UZgW/owvq/yRVyxfhsat2LUAuhVrHrLZm2SxHWTdi8U3X?=
 =?us-ascii?Q?i3ISnmPJYXt3au/5hlfGgZHRgYAtsX76cIQc28IvbVcHF8WAW7Z6MdHtXZM0?=
 =?us-ascii?Q?AvFjIBN3qQ7aUnn8v/8fMuK5pDePSsCk6JQPjBLtr8mjie1Jp0cGTQaNZMP8?=
 =?us-ascii?Q?vgNnGYwABV60fT8zaqZMGeaYadpva1oy+mhashngQsnaZ3gCAWpC6CbjlPpi?=
 =?us-ascii?Q?hiHI4m2HHcsAe4j+F/1GVmdxZIc+5MjF5+I/wlxWsPS7n7I2xtVozDJO+Xco?=
 =?us-ascii?Q?c3bbmUUmp/bWSOSTDZxFEuK4/zMn9Ic4k7IHI/4vKs/Ps2eEFalz/eh1zu1m?=
 =?us-ascii?Q?XPmluKWaqT4J6W1rlsewlHX8GchhfOpjAijS1G1xD0IpltoZIy8Pv84n7/qw?=
 =?us-ascii?Q?6CXq/bOV5YWxQHiAgYoWak0xua1XSCLsTzWEow1xcKM0uCtTobFbXI9HmMTf?=
 =?us-ascii?Q?7LLnBNa6zCkeeWKR4GDz5Mu+0DTUlITReWKY52C7gLPakCfsAuSL7+loO5Jy?=
 =?us-ascii?Q?BmVZfsoH+60LumCePzEX6E29lcQmx9QAMrcSg/DVB2kUKCsyG95UPbl34QG8?=
 =?us-ascii?Q?zEOQgfbXCeGTdMFt/X4mLFci8lz1NtxPsAt+wk6tgbBnfwP2lHjYhyrdDGYn?=
 =?us-ascii?Q?rqncYOTpDpt1yA8r+EgBzJcVmgbMWgWacTqkWlR7M0pMYUCgb618Hd/PWJen?=
 =?us-ascii?Q?HHKLBOo/lreqyM8XeAjPnNC+denI/+i6lxpiuKzQTAWMH3dzg9FcbuihtF6b?=
 =?us-ascii?Q?u6fzFWa0a0w0d1CnusghTzZyXt3I8AvkRxW/M+hkeQxAuIYzQlkPSQXl6Oyp?=
 =?us-ascii?Q?2S5etYUJOHIo/XFFYmJYJMcQ5LwLDl4x9VxqKiLDy/fZlR+GODqXrS/oBCNu?=
 =?us-ascii?Q?DjB6sFq6eo+85Qfpow3ML7V9WV9mCLvFp5y1Pf1CUiReIKRC828FPDEhEL8H?=
 =?us-ascii?Q?vwJNMZ3/1spPgKxZK7a1ngOmMYL91uAQ2OQ4SnLzd/+P5wsbZ5tOgo0FuBFs?=
 =?us-ascii?Q?dZzNYEtDqoNm/px9s0LV+JFH6g+JcEwB1NiByguTeHqL98wB4NZ6/YZGygkd?=
 =?us-ascii?Q?OgxQQO4h8b65hX9958lPKDezmzNlhxWqOnxo0QiIKH/hNJASYvws9aj66E1J?=
 =?us-ascii?Q?GGzE0KimyBqjRtZpFBjYdVIWfzFpnTfBi+JKF57FZv/w9aIsW4nvMNv5k/MU?=
 =?us-ascii?Q?1ys8uKhSQuHzv9FPFAXTaigqgofNpsm2QHcbxsBjkb8HiZXxySKInHDqskMx?=
 =?us-ascii?Q?T8iwV1YDg/Fw2N2O5ttBxB49yI6Q8aevYYHlJYqZLNHXbsx6LSQXoDdaBN0N?=
 =?us-ascii?Q?oOhOuL8a2+NG4JSwojJLSzq81BFT8JPfKLW24Lp7T26Vq6qEYCP5imWLxQHI?=
 =?us-ascii?Q?+5exkIJrzT97rRtLS3sPIMw7wItm7Cnh/Q4zzO5KWDssW+vOq4rRnON9I/sb?=
 =?us-ascii?Q?4BecqIcnNomzLUKwLHmeolgUZshSNnqVu0rorz+fEG388jDH/iJ863gEVc9W?=
 =?us-ascii?Q?ITw0iopsTbVAUo365O+U69mQ9yL/nLetF2mpZ34xx92DJtz+bKy1phyYC5C4?=
 =?us-ascii?Q?GoZKdLgQzspdADKo8G+7Zpy5sQBy8o+g8JacNEQw1qhhKHmiv3vVmmqaKQN2?=
 =?us-ascii?Q?Ip0NjkpXwjVBprryY6V2JFhYSehySNZ+H9p9TGyLhMd091Do9+Mu+4ZA0O+C?=
 =?us-ascii?Q?q+S2M6OJFsWSqcchY2nHuhLFzjh+bJDevxRY3SVRHuT0AF1jN98bp8JbtlP8?=
 =?us-ascii?Q?Eudm6jR1fxfQ3K3FOrIT/LC48VDdxJE=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a6682c66-bdb1-49d8-cb49-08da1defc728
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Apr 2022 08:21:30.8793
 (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: sgOZvIfiZD16K6AYNpivlX0cSv6VMz8w+ohSMZ9tdn8MQ60hu4xvYbWX1PYJKKjy0pf9Kv5BSG/Mmoj8HNj7ngA1NszbKLrkKMccnsE1HtA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB5115
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.486,18.0.858
 definitions=2022-04-14_02:2022-04-13,2022-04-14 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 suspectscore=0
 malwarescore=0 mlxlogscore=999 spamscore=0 adultscore=0 phishscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2202240000 definitions=main-2204140045
X-Proofpoint-ORIG-GUID: ML6XTp3WzEdh9h39wVyj3KTw-1Ws7Xw-
X-Proofpoint-GUID: ML6XTp3WzEdh9h39wVyj3KTw-1Ws7Xw-
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        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

On Thu, Apr 14, 2022 at 09:31:56AM +0200, Oliver Neukum wrote:
> 
> 
> On 13.04.22 17:32, Dan Carpenter wrote:
> >
> > Bug: buffer partially filled.  Information leak.
> >
> > If you return the bytes then the only correct way to write error
> > handling is:
> >
> > 	if (ret < 0)
> > 		return ret;
> > 	if (ret != size)
> > 		return -EIO;
> >
> You have to make up your mind on whether you ever need to read
> answer of a length not known before you try it. The alternative of
> passing a pointer to an integer for length is worse.

How is it worse?  Can you give an example, so I will write a static
checker rule for it?

There used to be more APIs that consistently caused bug after bug where
we mixed positives success values with negative error codes.  We
converted some bad offenders to return the positive as a parameter
and I was really happy about that.

Another example I used to see a lot is request_irq() saved to an
unsigned.  These days I think GCC warns about that?  Maybe the build
bots get to it before I do.

regards,
dan carpenter