aws-sdk-ruby Aws::S3::Object#presigned_ur のバグレポート - 修正済み

hiboma.hatenadiary.jp

このエントリで aws-sdk-ruby のバグレポートを出したことを記したが、既に修正されて aws-sdk-core (3.17.1) がリリースされている

github.com

めでたし

感想

  • aws-sdk-ruby で問題をレポートして「バグ」と 見なされた場合、数日で修正が入り対応が速い
  • JSON を喰わせてテストコードを生成してるんだなぁ

aws-sdk-ruby Aws::S3::Object#presigned_ur のバグレポート

aws-sdk-rubyAws::S3::Object#presigned_url を呼び出す際に :get + :reponse_expires を指定すると、署名付きURLを生成せずに NotImplementedError を raise してしまうのに遭遇した

どのようなコードなんですか?

#!/usr/bin/env ruby

require 'aws-sdk-s3'

s3 = Aws::S3::Resource.new(
  access_key_id: '***',
  secret_access_key: '***',
)

obj = s3.bucket('examplebucket').object('test.txt')
puts obj.presigned_url(:get, response_expires: Time.now)

このコードを実行すると NotImplementedError を raise する。

「バグ」なんですか?

レポートを出す前に、SDK の使い方を間違ってるという悲しい凡ミスではないことを確かめないといけない

ソースとドキュメントをきっちり調べた結果、 正式にサポートされているはずのパラメータだと確信を得て、issue でバグレポートを出した。

github.com

返信がついて「使い方は間違ってないね」とのことで、 Bug として ACK された

例外の発生箇所

問題となる例外をあげているコードは以下のようなコードだ

        def build_part(shape_ref, param_value)
          case shape_ref.shape # 👈
          # supported scalar types
          when StringShape, BooleanShape, FloatShape, IntegerShape, StringShape
            param_name = shape_ref.location_name
            "#{param_name}=#{escape(param_value.to_s)}"
          when MapShape
            if StringShape === shape_ref.shape.value.shape
              query_map_of_string(param_value)
            elsif ListShape === shape_ref.shape.value.shape
              query_map_of_string_list(param_value)
            else
              msg = "only map of string and string list supported"
              raise NotImplementedError, msg
            end
          when ListShape
            if StringShape === shape_ref.shape.member.shape
              list_of_strings(shape_ref.location_name, param_value)
            else
              msg = "Only list of strings supported, got "
              msg << shape_ref.shape.member.shape.class.name
              raise NotImplementedError, msg
            end
          else
            raise NotImplementedError # 🔥
          end
        end`

再現コードを実行すると、👈 で指す値が TimestampShape となるが、 case 〜 when で TimestampShape を扱わないので、🔥 で例外となる

テストすれば気がつきそうだが、: reponse_expires を指定した場合のテストケースが無いので、ただ単に実装漏れなんじゃないか? という

aws-sdk-ruby -ドキュメント の fix typo

ドキュメントを直す小粒な PR を出してマージされた

github.com

Aws::Sigv4::Signer は presign_url メソッドを持つのだが、ドキュメントで presigned_url になっていたのを直しただけ

小さな違いだがドキュメントを参照している際に混乱する


ところで、Aws::Sigv4::Signer を利用する上位のオブジェクトである Aws::S3::Object は presigned_url メソッドを持っているので ややこしい

f:id:hiboma:20180314125004p:plain

SDK の開発者/メンテナも両者をテレコにしてしまったのではないだろうか ?

ClamAV - clamd の OnAccessExtraScanning 機能のメモリリークをレポートした

ClamAV - clamd の OnAccessExtraScanning を使う際にメモリリークする不具合を見つけたのでバグレポートをだした

https://bugzilla.clamav.net/show_bug.cgi?id=12048

fix が入る前に公表してよいものか?

当初は 🔑 付きの issue として閲覧が制限されていたが、 担当者によって public な issue に変えられた。脆弱性としては扱われなかったようだ。

よって、本エントリで公言しても問題なかろうと判断している。

サマリ

環境や再現手順の詳細はリンク先を参照して欲しい。 このエントリではサマリを書いておこう

OnAccessExtraScanning とは ?

clamd で OnAccessExtraScanning yes にすると OnAccessIncludePathディレクトリ以下をオンアクセススキャンの対象とすることができ。 OnAccessIncludePathディレクトリ配下に新たなファイルやサブディレクトリを作る/移動/ リネームすると、 clamd が変更を検知してくれてスキャンしてくれるのだ

どういう不具合ですか?

  • clamd は動的にスキャン対象を追加する処理を pthread_create(3) して別スレッドに処理を委譲する設計としている
  • その際に pthread_attr_setdetachstate(..., PTHREAD_CREATE_JOINABLE) を指定してスレッドを作る
  • が、生成したスレッドを pthread_join(3) しないため pthread_create 内部で割り当てたメモリリージョンがリークする

どうやったら直りそうですか?

  1. pthread_join(3) しない設計のままメモリリークを解決するなら PTHREAD_CREATE_DETACHED を指定すべきだろう
  2. pthread_join(3) したまま設計を変える。1よりも粒度の大きい変更になるだろう
  3. そもそも スレッド使う設計をやめる

ぱっと、3択が考えられた。2 と 3 はちと粒度が大きすぎて自分が fix 案を提案できる感じではなかった。

最初に 1 を進言してみて最初は担当者にも同意してもらえていたが、問題を起こすことが発覚して 2 を選択する計画にしたようだ。PTHREAD_CREATE_DETACHED に変えてメモリリークを直すことだけにフォーカスしていて、副作用をだしていたことまでは把握できなかった。

0.100.0 を飛ばして 0.100.1 で fix されるというステータスになっているのが今ここのステータス

感想

  • 再現手順が単純なステップだったので、突き止めるのもスムーズに行った。
  • 不具合の再現を取っていて困ったのは、再現を取る中で 別の メモリリークが起きるのと clamd が SIGABORT するケースも併発しており、これらをどうやって切り離してレポートを書くかだった。(1レポートでは1つの不具合にフォーカスした方がよいだろうという考え)

なお、 調査は業務の一貫として行っております 😊

入間〜荒サイ 🚲

春眠 暁を Dura Ace

f:id:hiboma:20180312233621p:plain

のんびりペースで入間川まで出て、ほんのり追い風におされながら高速巡航で荒サイを下って終わり。1月、2月もほぼ毎週末走ってはいたけど、衰えてしまって長い距離を走れるまでになってない

続きを読む

pidof の -S / --separtor オプション で複数プロセスに strace する

procps の ML を眺めていたら便利そうな新機能がマージされていた

gitlab.com

$ ./pidof -h

Usage:
 lt-pidof [options] [program [...]]

Options:
 -s, --single-shot         return one PID only
 -c, --check-root          omit processes with different root
 -x                        also find shells running the named scripts
 -o, --omit-pid <PID,...>  omit processes with PID
 -S, --separator SEP       use SEP as separator put between PIDs 👈
 -h, --help     display this help and exit
 -V, --version  output version information and exit

For more details see pidof(1).

誰に便利って、頻繁に strace 使う人向け (コミットの説明にもまんま例が書いてある)

I frequency use pidof command with strace system call tracer.
strace can trace MULTIPLE processes specified with "-p $PID"
arguments like:

      strace -p 1 -p 1030 -p 3043

Sometimes I want to do as following

      strace -p $(pidof httpd)

However, above command line doesn't work because -p option
is needed for specifying a pid. pidof uses a whitespace as
a separator. For passing the output to strace, the separator
should be replaced with ' -p '.

This maybe not a special to my use case.

This commit introduces -S option that allows a user to specify a
separator the one wants.

    $ ./pidof bash
    ./pidof bash
    24624 18790 12786 11898 11546 10766 7654 5095
    $ ./pidof -S ',' bash
    ./pidof -S ',' bash
    24624,18790,12786,11898,11546,10766,7654,5095
    $ ./pidof -S '-p ' bash
    ./pidof -S '-p ' bash
    24624-p 18790-p 12786-p 11898-p 11546-p 10766-p 7654-p 5095
    $ ./pidof -S ' -p ' bash
    ./pidof -S ' -p ' bash
    24624 -p 18790 -p 12786 -p 11898 -p 11546 -p 10766 -p 7654 -p 5095
    $ strace -p $(./pidof -S ' -p ' bash)
    strace -p $(./pidof -S ' -p ' bash)
    strace: Process 24624 attached
    strace: Process 18790 attached
    strace: Process 12786 attached
    ...

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
 parent c9be22a8 master

こんな感じで strace で複数プロセスに一度でアタッチできる! あーべんり〜 わかる〜

[vagrant@localhost procps]$ sudo strace -p $(./pidof nginx -S ' -p ')
strace: Process 9621 attached
strace: Process 9620 attached
strace: Process 9619 attached
[pid  9619] rt_sigsuspend([], 8 <unfinished ...> 👈
[pid  9620] epoll_wait(9,  <unfinished ...> 👈
[pid  9621] epoll_wait(11,  👈

複数のコマンドを組み合わせて同様のことを実現するテクニックはあるのだが、 シンプルな手段があれば越したことはない

procps-ng: sysctl の Segmentation Fault を報告 、の続き

hiboma.hatenadiary.jp

上記のエントリにも書いたのだが、2ヶ月ほど前に procps-ng にバグレポートを投げていたのにようやく反応をもらえて、速攻で fix された

gitlab.com

gitlab.com

なかなかのんびりしているプロジェクトなのだな。また何か見つけたら貢献していこう


(追記)

私が issue を立てる前に、バグを作ってしまった作者自信から修正パッチが ML に投稿されていたとのこと。が、見過ごされていたらしい 😭

running sysctl as non-privileged user causes segmentaion fault (#76) · Issues · procps-ng / procps · GitLab

www.freelists.org